[telemetry] Add support for composable process dumps in memory-infra

tools/telemetry/telemetry/timeline/memory_dump_event.py

Index: tools/telemetry/telemetry/timeline/memory_dump_event.py
diff --git a/tools/telemetry/telemetry/timeline/memory_dump_event.py b/tools/telemetry/telemetry/timeline/memory_dump_event.py
index fff2a677588e98427422ed09fdb16514454a119a..694e99fc59ef3d0f819a3eb1c120ead12538b5a3 100644
--- a/tools/telemetry/telemetry/timeline/memory_dump_event.py
+++ b/tools/telemetry/telemetry/timeline/memory_dump_event.py
@@ -146,25 +146,40 @@ class ProcessMemoryDumpEvent(timeline_event.TimelineEvent):
     has_mmaps: True if the memory dump has mmaps information. If False then
         GetMemoryUsage will report all zeros.
   """
-  def __init__(self, process, event):
-    assert event['ph'] == 'v' and process.pid == event['pid']
+  def __init__(self, process, dump_events):
+    assert dump_events
 
-    super(ProcessMemoryDumpEvent, self).__init__(
-        'memory', 'memory_dump', event['ts'] / 1000.0, 0.0)
+    start_time = min(event['ts'] for event in dump_events) / 1000.0
+    duration = max(event['ts'] for event in dump_events) / 1000.0 - start_time
+    super(ProcessMemoryDumpEvent, self).__init__('memory', 'memory_dump',
+                                                 start_time, duration)
 
     self.process = process
-    self.dump_id = event['id']
+    self.dump_id = dump_events[0]['id']
+
+    raw_allocator_dumps = {}
+    raw_vm_regions = []
+    for event in dump_events:
+      assert (event['ph'] == 'v' and self.process.pid == event['pid'] and
+          self.dump_id == event['id'])

[perezju, 2016/01/12 15:53:37]

nit: I think that |event| and |self| should be aligned here.

[ssid, 2016/01/12 17:13:17]

Done.

+      try:
+        raw_allocator_dumps.update(event['args']['dumps']['allocators'])
+      except KeyError:
+        pass  # It's ok if any of those keys are not present.
+      try:
+        vm_regions = event['args']['dumps']['process_mmaps']['vm_regions']
+        assert not raw_vm_regions
+        raw_vm_regions += vm_regions

[perezju, 2016/01/12 15:53:37]

This bit of code seems a bit odd, having this vm_regions variable to momentarily
hold the value before the assert. But honestly can't think of a much better way
to do this.

Maybe just call the variable something more generic like "value" to suggest this
is just a temporary variable not used elsewhere in the method.

Also, since you add to the list only once, maybe initialize raw_vm_regions to
None; and when (if) you find a value then just set it as is (instead of
appending to the empty list)?

[ssid, 2016/01/12 17:13:17]

Done.
This would need a if condition when i use it. I used = to replace the empty
list. 

+      except KeyError:
+        pass  # It's ok if any of those keys are not present.
 
-    try:
-      allocators_dict = event['args']['dumps']['allocators']
-    except KeyError:
-      allocators_dict = {}
     self._allocators = {}
     parent_path = ''
     parent_has_size = False
-    for allocator_name, size_values in sorted(allocators_dict.iteritems()):
-      if ((allocator_name.startswith(parent_path) and parent_has_size)
-          or allocator_name.startswith('global/')):
+    for allocator_name, size_values in sorted(
+        raw_allocator_dumps.iteritems()):

[perezju, 2016/01/12 15:53:37]

nit: why not keep the old names (allocators_dict and vm_regions)? / feel free to
ignore

[ssid, 2016/01/12 17:13:17]

Done.

+      if ((allocator_name.startswith(parent_path) and parent_has_size) or
+          allocator_name.startswith('global/')):
         continue
       parent_path = allocator_name + '/'
       parent_has_size = 'size' in size_values['attrs']
@@ -173,8 +188,8 @@ class ProcessMemoryDumpEvent(timeline_event.TimelineEvent):
       # For 'gpu/android_memtrack/*' we want to keep track of individual
       # components. E.g. 'gpu/android_memtrack/gl' will be stored as
       # 'android_memtrack_gl' in the allocators dict.
-      if (len(name_parts) == 3 and allocator_name == 'gpu'
-          and name_parts[1] == 'android_memtrack'):
+      if (len(name_parts) == 3 and allocator_name == 'gpu' and
+          name_parts[1] == 'android_memtrack'):
         allocator_name = '_'.join(name_parts[1:3])
       allocator = self._allocators.setdefault(allocator_name, {})
       for size_key, size_value in size_values['attrs'].iteritems():
@@ -185,15 +200,11 @@ class ProcessMemoryDumpEvent(timeline_event.TimelineEvent):
     try:
       self._allocators['malloc']['size'] -= self._allocators['tracing']['size']
     except KeyError:
-      pass # it's ok if any of those keys are not present
+      pass  # It's ok if any of those keys are not present.
 
+    self.has_mmaps = bool(raw_vm_regions)
     self._buckets = {}
-    try:
-      vm_regions = event['args']['dumps']['process_mmaps']['vm_regions']
-    except KeyError:
-      vm_regions = []
-    self.has_mmaps = bool(vm_regions)
-    for vm_region in vm_regions:
+    for vm_region in raw_vm_regions:
       self._AddRegion(vm_region)
 
   @property
@@ -259,8 +270,10 @@ class ProcessMemoryDumpEvent(timeline_event.TimelineEvent):
       if 'memtrack_pss' in values:
         usage[name] = values['memtrack_pss']
     if self.has_mmaps:
-      usage.update((key, self.GetMemoryValue(*value))
-                   for key, value in MMAPS_METRICS.iteritems())
+      for key, value in MMAPS_METRICS.iteritems():
+        result = self.GetMemoryValue(*value)
+        if result:
+          usage[key] = result
     return usage
 
 
@@ -297,7 +310,7 @@ class GlobalMemoryDump(object):
 
   @property
   def end(self):
-    return self._process_dumps[-1].start
+    return max(dump.end for dump in self._process_dumps)

[perezju, 2016/01/12 15:53:37]

Add a test that _would_ have failed if you were forget making this change.

[ssid, 2016/01/12 17:13:17]

Done.

 
   @property
   def duration(self):