Add a --startup option to generate combined traces for startup.

tools/profile_chrome_startup.py

+#!/usr/bin/env python
+#
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import logging
+import optparse
+import os
+import re
+import sys
+import time
+import webbrowser
+
+sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir,
+                             'build', 'android'))
+
+from profile_chrome import controllers
+from profile_chrome import flags
+from profile_chrome import profiler
+from profile_chrome import systrace_controller
+from profile_chrome import ui
+from pylib import android_commands
+from pylib import flag_changer
+from pylib.device import device_utils
+from pylib.perf import cache_control
+
+class ChromeStartupTracingController(controllers.BaseController):

[Sami, 2015/01/27 17:08:33]

Please add a test for this like we do with the other controllers.

[Benoit L, 2015/01/27 18:41:41]

Done.

+  def __init__(self, device, package_info, cold, url):
+    self._device = device
+    self._package_info = package_info
+    self._cold = cold
+    self._url = url
+    self._trace_file = None
+    self._trace_finish_re = re.compile(r' Completed startup tracing to (.*)')
+    self._device.old_interface.StartMonitoringLogcat(clear=False)
+
+  def __repr__(self):

[pasko, 2015/01/27 17:00:24]

is this necessary?

[Sami, 2015/01/27 17:08:33]

CaptureProfile uses it to print out the type of trace we are recording. Please
use single quotes though.

[Benoit L, 2015/01/27 18:41:41]

Done.

[Benoit L, 2015/01/27 18:41:42]

No, but prettier.
In profiler.py, this gets printed:

  trace_type = ' + '.join(map(str, controllers))

So having a nice representation helps (and this is what is done by the other
controllers).

+    return "Browser Startup Trace"
+
+  def _SetupTracing(self):
+    changer = flag_changer.FlagChanger(
+        self._device, self._package_info.cmdline_file)
+    changer.AddFlags(['--trace-startup'])
+    self._device.old_interface.CloseApplication(self._package_info.package)
+    if self._cold:
+      self._device.old_interface.EnableAdbRoot()
+      cache_control.CacheControl(self._device).DropRamCaches()
+    self._device.old_interface.StartActivity(
+        package=self._package_info.package,
+        activity=self._package_info.activity,
+        data=self._url,
+        extras={'create_new_tab' : True})
+
+  def StartTracing(self, interval):
+    self._SetupTracing()
+    self._device.old_interface.SyncLogCat()
+
+  def StopTracing(self):

[Sami, 2015/01/27 17:08:32]

Can you make sure to remove the --trace-startup flag from the command line file?
It'll be pretty surprising for the user if it persists.

[Benoit L, 2015/01/27 18:41:41]

Indeed, thank you for pointing this out.
Done.

[pasko, 2015/01/28 12:26:03]

What happens if the tracing is not stopped properly? Say, a device would
disconnect, we throw an exception and exit without going through StopTracing.

It seems reasonable to require all performance-related scripts to push their own
flags file on the start to make the performance results more reproducible. It is
OK to perform a 'best effort restore' of course.

There is an alternative of always handling exceptions properly in telemetry, but
I don't think we are ready to go that path at the moment.

So, how about a TODO for the next CL to push the 'canonical' perf flags to the
device before running any benchmarks?

[Sami, 2015/01/28 12:44:14]

adb_profile_chrome isn't just used for running benchmarks, so I don't think it
should touch the command line unnecessarily. Before this patch it doesn't event
launch the activity. Telemetry seems like the better place for that sort of
stuff, and we already do a lot of provisioning there.

[pasko, 2015/01/28 12:50:53]

Agreed, when startup is not affected, pushing the flags is unnecessary and may
harm, but for adb_profile_chrome_startup having results offset by incorrect
startup settings may be unpleasant too.

Oh, by the way, Benoit, please change the issue description to mention
adb_profile_chrome_startup instead of --startup.

+    self._trace_file = self._device.old_interface.WaitForLogMatch(
+        self._trace_finish_re, None, timeout=120).group(1)
+
+  def PullTrace(self):
+    # Wait a bit for the browser to finish writing the trace file.
+    time.sleep(3)
+    trace_file = self._trace_file.replace('/storage/emulated/0/', '/sdcard/')
+    host_file = os.path.join(os.path.curdir, os.path.basename(trace_file))
+    self._device.PullFile(trace_file, host_file)
+    return host_file
+
+
+def _CreateOptionParser():
+  parser = optparse.OptionParser(description='Record about://tracing profiles '
+                                 'from Android browsers startup, combined with '
+                                 'Android systrace. See http://dev.chromium.org'
+                                 '/developers/how-tos/trace-event-profiling-'
+                                 'tool for detailed instructions for '
+                                 'profiling.')
+  parser.add_option('--kind', help='Kind of startup. Possible values are "warm"'
+                    ' and "cold". The "warm" start does not perform special '
+                    'steps, while the "cols" flushes the OS page cache before '

[pasko, 2015/01/27 17:00:24]

s/cols/cold/

this option seems to be ignored and replaced with --cold?

[Benoit L, 2015/01/27 18:41:41]

You're right, oops.
Done.

+                    'start. Note that "cold" requires a device with root '
+                    'access.', default="warm", choices=['warm', 'cold'])
+  parser.add_option('--url', help='URL to visit on startup. Default: '
+                    'https://wwww.google.com', default='https://www.google.com',
+                    metavar='URL')
+  parser.add_option('--cold', help='Flush the OS page cache before starting the'
+                    ' browser. Note that this require a device with root '
+                    'access.', default=False, action='store_true')
+  parser.add_option_group(flags.SystraceOptions(parser))
+  parser.add_option_group(flags.OutputOptions(parser))
+
+  browsers = sorted(profiler.GetSupportedBrowsers().keys())
+  parser.add_option('-b', '--browser', help='Select among installed browsers. '

[pasko, 2015/01/27 17:00:24]

maybe these options could also be factored out?

[Benoit L, 2015/01/27 18:41:41]

It seemed to be more trouble than necessary, but I can change it.

[pasko, 2015/01/28 12:26:04]

Ack, good enough for now.

+                    'One of ' + ', '.join(browsers) + ', "stable" is used by '
+                    'default.', type='choice', choices=browsers,
+                    default='stable')
+  parser.add_option('-v', '--verbose', help='Verbose logging.',
+                    action='store_true')
+  parser.add_option('-z', '--compress', help='Compress the resulting trace '
+                    'with gzip. ', action='store_true')
+  return parser
+
+
+def main():
+  parser = _CreateOptionParser()
+  options, _ = parser.parse_args()
+
+  if options.verbose:
+    logging.getLogger().setLevel(logging.DEBUG)
+
+  devices = android_commands.GetAttachedDevices()
+  if len(devices) != 1:
+    logging.error('Exactly 1 device must be attached.')
+    return 1
+  device = device_utils.DeviceUtils(devices[0])
+  package_info = profiler.GetSupportedBrowsers()[options.browser]
+
+  if options.systrace_categories in ['list', 'help']:
+    ui.PrintMessage('\n'.join(
+        systrace_controller.SystraceController.GetCategories(device)))
+    return 0
+  systrace_categories = options.systrace_categories.split(',')
+
+  enabled_controllers = []
+  # Enable the systrace and chrome controller. The systrace controller should go
+  # first because otherwise the resulting traces miss early systrace data.
+  if systrace_categories:
+    enabled_controllers.append(systrace_controller.SystraceController(
+        device, systrace_categories, False))
+  enabled_controllers.append(
+      ChromeStartupTracingController(device, package_info, options.cold,
+                                     options.url))
+  if options.output:
+    options.output = os.path.expanduser(options.output)
+  result = profiler.CaptureProfile(enabled_controllers, 0,
+                                   output=options.output,
+                                   compress=options.compress,
+                                   write_json=options.json)
+  if options.view:
+    if sys.platform == 'darwin':
+      os.system('/usr/bin/open %s' % os.path.abspath(result))
+    else:
+      webbrowser.open(result)
+
+
+if __name__ == '__main__':
+  sys.exit(main())