[Telemetry] Fix idle wakeup reporting in the face of dead processes

tools/telemetry/telemetry/core/platform/timeline_objects.py

Index: tools/telemetry/telemetry/core/platform/timeline_objects.py
diff --git a/tools/telemetry/telemetry/core/platform/timeline_objects.py b/tools/telemetry/telemetry/core/platform/timeline_objects.py
new file mode 100644
index 0000000000000000000000000000000000000000..b27bc268e1a041ad7f44bb689c19c62bfef3b014
--- /dev/null
+++ b/tools/telemetry/telemetry/core/platform/timeline_objects.py
@@ -0,0 +1,69 @@
+# Copyright 2014 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 json
+
+from telemetry.timeline import timeline_data
+
+
+class IdleStatsData(timeline_data.TimelineData):

[tonyg, 2014/08/24 16:53:44]

I think this implementation should live in
tools/telemetry/telemetry/timeline/idle_wakeup_timeline_data.py and be named
IdleWakeupTimelineData. WDYT?

Another thought is that no code in this file other than the naming is actually
specific to idle wakeups. What if we called it something like
ProcessStatisticTimelineData and either passed a string to the ctor indicating
the type or else made trivial subclasses for each time. I mention that because
the dicts returned by Browser.cpu_stats, io_stats, memory_stats have the same
problem.

[nednguyen, 2014/08/25 15:42:00]

I think this patch is using timeline_data interface for a wrong purpose.
Traditionally, it has been used as a thin wrapper for raw data that can be
consumed by different importer method, it doesn't mean to contain helper methods
for processing the specific raw data.

Nat is working on a patch to remove timeline_data interface, and move these data
to TraceValuePart:

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...

I think the right approach would be: platform gives the raw data, importer
processes raw data and augment them to model.

[jeremy, 2014/08/26 14:14:16]

Done with trivial subclasses, I haven't moved this into timeline pending the
dust settling from the changes Ned mentioned.

+  """Hold idle wakeup counts for one or more processes.
+
+  This object can hold data for more than one pid by adding another object."""
+
+  def __init__(self, pid, idle_wakeup_count):
+    super(IdleStatsData, self).__init__()
+    assert idle_wakeup_count >= 0
+    self._idle_wakeup_count_by_pid = {pid : idle_wakeup_count}

[tonyg, 2014/08/24 16:53:44]

nit: no space before :

[jeremy, 2014/08/26 14:14:16]

Done.

+
+  def __sub__(self, other):
+    """The results of subtraction is an object holding only the pids contained
+    in both |self| and |other|.
+
+    The motivation is that some processes may have died between two consecutive
+    measurements. The desired behavior is to only make calculations based on
+    the processes that are still alive at the end of the second measurement."""

[tonyg, 2014/08/24 16:53:44]

What about new processes (those in the second, but not the first)? Should we
include all wakeups for the lifetime of those processes?

I asked b/c this'll be common for navigations which create a new renderer.

[jeremy, 2014/08/26 14:14:16]

Done.

+    # pylint: disable=W0212
+    ret = IdleStatsData(0, 0)
+    my_dict = self._idle_wakeup_count_by_pid
+
+    intersecting_pids = (set(my_dict.keys()) &
+        set(other._idle_wakeup_count_by_pid.keys()))
+
+    ret._idle_wakeup_count_by_pid = (
+        {k: my_dict[k] - other._idle_wakeup_count_by_pid.get(k, 0) for
+            k in intersecting_pids})
+    return ret
+
+  def __add__(self, other):
+    """The result contains pids from both |self| and |other|, if duplicate
+    pids are found between objects, an error will occur. """
+    # pylint: disable=W0212
+    intersecting_pids = (set(self._idle_wakeup_count_by_pid.keys()) &
+        set(other._idle_wakeup_count_by_pid.keys()))
+    assert len(intersecting_pids) == 0
+
+    ret = IdleStatsData(0, 0)
+    ret._idle_wakeup_count_by_pid = {}
+    ret._idle_wakeup_count_by_pid.update(self._idle_wakeup_count_by_pid)
+    ret._idle_wakeup_count_by_pid.update(other._idle_wakeup_count_by_pid)
+    return ret
+
+  @property
+  def idle_wakeup_count_by_pid(self):
+    return self._idle_wakeup_count_by_pid
+
+  def IdleWakeupCount(self):
+    """Returns the sum of all idle wakeups contained by this object. """
+    return sum(self._idle_wakeup_count_by_pid.values())
+
+  def Serialize(self, f):
+    """Serializes the idle wakeup count to a file-like object."""
+    f.write('{"idleWakeupCount":')
+    json.dump(self.IdleWakeupCount(), f, indent=4)
+    f.write('}')
+
+  def EventData(self):
+    # TODO(jeremy): implement this when we have a corresponding importer setup.
+    pass