Telemetry: integrates memory_measurement with TCMalloc dumps

tools/telemetry/telemetry/core/platform/profiler/tcmalloc_heap_profiler.py

Index: tools/telemetry/telemetry/core/platform/profiler/tcmalloc_heap_profiler.py
diff --git a/tools/telemetry/telemetry/core/platform/profiler/tcmalloc_heap_profiler.py b/tools/telemetry/telemetry/core/platform/profiler/tcmalloc_heap_profiler.py
new file mode 100644
index 0000000000000000000000000000000000000000..5f7e767f104221a41ee4ebcba7b979b79f3d4d49
--- /dev/null
+++ b/tools/telemetry/telemetry/core/platform/profiler/tcmalloc_heap_profiler.py
@@ -0,0 +1,125 @@
+# Copyright (c) 2013 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 os
+import sys
+
+from telemetry.core.chrome import adb_commands
+from telemetry.core.platform import profiler
+
+# Enviroment variables to (android properties, default value) mapping.
+_ENV_VARIABLES = {
+  'HEAP_PROFILE_TIME_INTERVAL': ('heapprof.time_interval', 20),
+  'HEAP_PROFILE_MMAP': ('heapprof.mmap', 1),
+  'DEEP_HEAP_PROFILE': ('heapprof.deep_heap_profile', 1),
+}
+
+class _TCMallocHeapProfilerAndroid(profiler.Profiler):

[tonyg, 2013/05/14 16:52:44]

Can these just implement object instead of Profiler and drop the is_supported
and name methods? It seems like TCMallocHeapProfiler has the Profiler interface
covered.

[bulach, 2013/05/14 17:54:47]

oh, much nicer! done.

+  """An internal class to set android properties and fetch dumps from device."""
+
+  _DEFAULT_DEVICE_DIR = '/data/local/tmp/heap/'
+
+  def __init__(self, browser_backend, _, output_path):
+    super(_TCMallocHeapProfilerAndroid, self).__init__(output_path)
+    self._browser_backend = browser_backend
+
+    properties = dict(_ENV_VARIABLES)
+    properties['HEAPPROFILE'] = ('heapprof',
+        self._DEFAULT_DEVICE_DIR + 'dmprof')
+
+    self._SetDeviceProperties(self._browser_backend.options.android_device,
+        properties)
+
+  def _SetDeviceProperties(self, android_device, properties):
+    adb = adb_commands.AdbCommands(android_device)

[tonyg, 2013/05/14 16:52:44]

I didn't double check, but can't we get an existing adb_commands object from the
platform_backend or something?

[bulach, 2013/05/14 17:54:47]

Done.

+    has_set_property = False
+    for values in properties.itervalues():
+      device_property = adb.RunShellCommand('getprop ' + values[0])
+      if (not device_property or len(device_property) != 1 or
+          not device_property[0].strip()):
+        print 'Setting device property ', values[0], values[1]

[tonyg, 2013/05/14 16:52:44]

logging.info?

[bulach, 2013/05/14 17:54:47]

Done.

+        adb.RunShellCommand('setprop ' + values[0] + ' ' + str(values[1]))
+        has_set_property = True
+    if has_set_property:
+      raise Exception('New device properties were set, run again.')

[tonyg, 2013/05/14 16:52:44]

Why do we have to run again? Can we do Browser.close() or something instead?

[bulach, 2013/05/14 17:54:47]

I think at this stage where the profiler has been created is already too late to
mess up with the browser...

[tonyg, 2013/05/14 18:54:13]

You are right, too bad.

+    adb.RunShellCommand('rm -rf ' + self._DEFAULT_DEVICE_DIR)
+    adb.RunShellCommand('mkdir -p ' + self._DEFAULT_DEVICE_DIR)
+    adb.RunShellCommand('chmod 777 ' + self._DEFAULT_DEVICE_DIR)
+
+  @classmethod
+  def name(cls):
+    return 'tcmalloc'
+
+  @classmethod
+  def is_supported(cls, options):
+    return True
+
+  def CollectProfile(self):
+    print 'TCMalloc dumps available in the device ', self._DEFAULT_DEVICE_DIR

[tonyg, 2013/05/14 16:52:44]

Is this print necessary since it will be pulled to the host?

[bulach, 2013/05/14 17:54:47]

Done.

+    adb = adb_commands.AdbCommands(self._browser_backend.options.android_device)

[tonyg, 2013/05/14 16:52:44]

Again, let's try to reuse adb_commands if possible.

[bulach, 2013/05/14 17:54:47]

Done.

+    adb.Adb().Adb().Pull(self._DEFAULT_DEVICE_DIR, self.output_path)
+    print 'TCMalloc dumps pulled to ', self.output_path
+
+
+class _TCMallocHeapProfilerLinux(profiler.Profiler):
+  """An internal class to set environment variables and fetch dumps."""
+
+  _DEFAULT_DIR = '/tmp/tcmalloc/'
+
+  def __init__(self, browser_backend, _, output_path):
+    super(_TCMallocHeapProfilerLinux, self).__init__(output_path)
+    self._browser_backend = browser_backend
+
+    env_vars = dict(_ENV_VARIABLES)

[tonyg, 2013/05/14 16:52:44]

What does the dict() wrapper do? Isn't it already a dict?

[bulach, 2013/05/14 17:54:47]

at some point I did that in the classmethod and didn't want to mess up with the
"singleton", so dict() makes a copy of it..
no longer necessary, removed here and above.

+    env_vars['HEAPPROFILE'] = ('heapprof', self._DEFAULT_DIR + 'dmprof')
+    self._CheckEnvironmentVariables(env_vars)
+
+  def _CheckEnvironmentVariables(self, env_vars):
+    msg = ''
+    for key, values in env_vars.iteritems():
+      if key not in os.environ:
+        msg += '%s=%s ' % (key, str(values[1]))
+    if msg:
+      raise Exception('Need enviroment variables, try again with:\n %s' % msg)
+    if not os.path.exists(os.environ['HEAPPROFILE']):

[tonyg, 2013/05/14 16:52:44]

Should we check isdir() as well?

[bulach, 2013/05/14 17:54:47]

adding an assertion below.

+      os.makedirs(os.environ['HEAPPROFILE'])
+
+  @classmethod
+  def name(cls):
+    return 'tcmalloc'
+
+  @classmethod
+  def is_supported(cls, options):
+    return True
+
+  def CollectProfile(self):
+    print 'TCMalloc dumps available ', os.environ['HEAPPROFILE']
+
+
+class TCMallocHeapProfiler(profiler.Profiler):
+  """A Factory to instantiate the platform-specific profiler."""
+  def __init__(self, browser_backend, _, output_path):
+    super(TCMallocHeapProfiler, self).__init__(output_path)
+    self._browser_backend = browser_backend
+    if self._browser_backend.options.browser_type.startswith('android'):
+      self._platform_profiler = _TCMallocHeapProfilerAndroid(
+          browser_backend, _, output_path)
+    else:
+      self._platform_profiler = _TCMallocHeapProfilerLinux(
+          browser_backend, _, output_path)
+
+  @classmethod
+  def name(cls):
+    return 'tcmalloc-heap'
+
+  @classmethod
+  def is_supported(cls, options):
+    if (sys.platform != 'linux2'):

[tonyg, 2013/05/14 16:52:44]

drop the parens. Also, I think we normally do startswith('linux') instead of ==
'linux2'.

[bulach, 2013/05/14 17:54:47]

Done.

[bulach, 2013/05/14 17:54:47]

Done.

+      return False
+    if options.browser_type.startswith('android'):

[tonyg, 2013/05/14 16:52:44]

Do we need to throw in an:

if options.browser_type.startswith('cros'):
  return False

[bulach, 2013/05/14 17:54:47]

Done.

+      return _TCMallocHeapProfilerAndroid.is_supported(options)
+    return _TCMallocHeapProfilerLinux.is_supported(options)

[tonyg, 2013/05/14 16:52:44]

These is_supported calls are no-ops. I'd just return True.

[bulach, 2013/05/14 17:54:47]

Done.

+
+  def CollectProfile(self):
+    self._platform_profiler.CollectProfile()