Move timeline and importers to use telemetry.value.TraceValue

tools/telemetry/telemetry/value/trace.py

+# 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
+import numbers
+
+def _ValidateRawData(raw):
+  if isinstance(raw, basestring):
+    return
+  if isinstance(raw, numbers.Number):
+    return
+

[nednguyen, 2014/08/05 14:34:34]

Can None be raw data?
For example: {1:None, 2:'foo'}

+  if isinstance(raw, list):
+    for r in raw:
+      _ValidateRawData(r)
+    return
+
+  if isinstance(raw, dict):
+    for k,v in raw.iteritems():
+      _ValidateRawData(k)
+      _ValidateRawData(v)
+    return
+
+  raise Exception('%s is not allowed in TraceValue', raw)

[nednguyen, 2014/08/05 14:34:34]

Throwing exception at the leaf level may make it hard to see what went wrong if
we have a huge dictionary.

+
+
+# TraceValues have a variety of events in them. These are called "parts" and
+# aare accessed by name, using the following fixed field names.

[nednguyen, 2014/08/05 14:34:34]

nit: are

+class TraceValuePart(object):

[chrishenry, 2014/08/05 23:44:15]

Just TracePart? TraceValuePart is a bit ambiguous.

+  def __init__(self, rawFieldName):
+    self.rawFieldName = rawFieldName

[chrishenry, 2014/08/05 23:44:15]

nit: raw_field_name

+
+  def __repr__(self):
+    return 'TraceValuePart("%s")' % self.rawFieldName
+
+CHROME_TRACE_PART = TraceValuePart('traceEvents')
+INSPECTOR_TRACE_PART = TraceValuePart('inspectorTimelineEvents')
+TAB_ID_PART = TraceValuePart('tabIds')
+
+ALL_TRACE_VALUE_PARTS = set([CHROME_TRACE_PART,
+                             INSPECTOR_TRACE_PART,
+                             TAB_ID_PART])
+
+class RawTraceValueHelpersMixin(object):

[nednguyen, 2014/08/05 14:34:34]

Don't you need __init__(self) to define self._raw_data?

[chrishenry, 2014/08/05 23:44:16]

Does TraceValue and Builder really need to mixin this? Maybe they can just be
free functions.

+  @property
+  def active_parts(self):
+    return set([p for p in ALL_TRACE_VALUE_PARTS
+                if p.rawFieldName in self._raw_data])
+
+  def HasEventsFor(self, part):
+    assert isinstance(part, TraceValuePart)
+    if part.rawFieldName not in self._raw_data:
+      return False
+    return len(self._raw_data[part.rawFieldName]) > 0
+
+  def GetEventsFor(self, part):
+    assert isinstance(part, TraceValuePart)
+    return self._raw_data[part.rawFieldName]
+
+  @property
+  def metadata_records(self):
+    part_field_names = set([p.rawFieldName for p in ALL_TRACE_VALUE_PARTS])

[slamm, 2014/08/12 23:11:59]

You can build the set with a generator expression instead of a list:

part_field_names = set(p.rawFieldName for p in ALL_TRACE_VALUE_PARTS)

+    for k,v in self._raw_data.iteritems():

[chrishenry, 2014/08/05 23:44:15]

nit: k, v (space after ,)

+      if k in part_field_names:
+        continue
+      yield {
+        'name' : k,

[chrishenry, 2014/08/05 23:44:15]

nit: no space before :

+        'value' : self._raw_data[v]

[nednguyen, 2014/08/05 14:34:34]

'value': v ?

+      }
+
+
+# TODO(nduca): Make this subclass value. Moving it was a huge enough patch
+# that the true subclassing will be done in a followup.
+class TraceValue(RawTraceValueHelpersMixin):
+  def __init__(self, raw_data=None):
+    """Creates a TraceValue from the given data.
+
+    NOTE: raw data must NOT include any non-primitive objects!
+    By design, TraceValue must contain only data that is BOTH serializable
+    to a file, AND restorable once again from that file into a TraceValue
+    without assistance from other classes.
+
+    raw_data can be any of the three standard trace_event formats:

[nednguyen, 2014/08/05 14:34:34]

So we will need something else for video and images?

+    1. Trace container format: a json-parseable dict.
+    2. A json-parseable array: assumed to be chrome trace data.
+    3. A json-parseable array missing the final ]: assumed to be
+       chrome trace data.
+    """
+    self._raw_data = {}
+    self._events_are_safely_mutable = False
+    if raw_data == None:
+      return
+
+    _ValidateRawData(raw_data)
+
+    if self._TryInitFromRaw(raw_data):
+      return
+
+    if not isinstance(raw_data, basestring):

[chrishenry, 2014/08/05 23:44:16]

I think it's clearer to refactor TryInitFromRaw and this next section into
something like:

# Parse into json object if raw_value is a string.
if isinstance(..., basestring):
  json_data = _ParseFromString(...)
  # The parsed data isn't shared with anyone else, so we mark this value as
safely mutable.
  self._events_are_safely_mutable = True
else:
  json_data = raw_data

if isinstance(json_data, dict):
  ...
elif isinstance(json_data, list):
  ...
else:
  raise Exception('Unrecognized...')

+      raise ValueError('Unrecognized data format')
+
+    if len(raw_data) == 0:
+      self._raw_data == {}

[slamm, 2014/08/12 23:11:59]

self._raw_data = {} ?
Same in _TryInitFromRaw.

+      return
+
+    if raw_data[0] == '[' and raw_data[-1] != ']':

[slamm, 2014/08/12 23:11:59]

Better?

if raw_data.startswith('[') and not raw_data.endswith(']'):
  if raw_data.endswith(','):
    raw_data = raw_data[:-1]
  raw_data += ']'

+      if raw_data[-1] == ',':
+        raw_data = raw_data[:-1] + ']'
+      else:
+        raw_data = raw_data + ']'
+
+    raw_data = json.loads(raw_data)
+    if self._TryInitFromRaw(raw_data):
+      self._events_are_safely_mutable = True
+      return
+
+    raise Exception('Unrecognized data format')
+
+  def _TryInitFromRaw(self, raw_data):
+    if isinstance(raw_data, dict):
+      self._raw_data = raw_data
+      return True
+
+    if isinstance(raw_data, list):
+      if len(raw_data) == 0:
+        self._raw_data == {}
+        return True
+
+      self._raw_data = {'traceEvents': raw_data}

[nednguyen, 2014/08/05 14:34:34]

How about self._raw_data = { CHROME_TRACE_PART.rawFieldName : raw_data}? 

+      return True
+
+    return False
+
+  def _SetFromBuilder(self, d):
+    self._raw_data = d
+    self._event_data_is_mutable = True

[chrishenry, 2014/08/05 23:44:16]

Unused? Or do you mean _events_are_safely_mutable?

+
+  @property
+  def events_are_safely_mutable(self):
+    """Returns true if the events in this value are completely sealed.
+
+    Some importers want to take complex fields out of the TraceValue and add
+    them to the model, changing them subtly as they do so. If the TraceValue was

[chrishenry, 2014/08/05 23:44:16]

Do these importers modify the raw data in this TraceValue? You mentioned below
that TraceValue is supposed to be immutable, is that not true then?

+    constructed with data that is shared with something outside the trace value,
+    for instance a test harness, then this mutation is unexpected. But, if the
+    values are sealed, then mutating the events is a lot faster.
+
+    We know if events are sealed if the value came from a string, or if the
+    value came from

[chrishenry, 2014/08/05 23:44:16]

incomplete sentence?

+    """
+    return self._events_are_safely_mutable
+
+  def Serialize(self, f, gzip_result=False):
+    """Serializes the trace result to a file-like object.
+
+    Always writes in the trace container format."""
+    assert not gzip_result, 'Not implemented'
+    json.dump(self._raw_data, f)
+
+
+class TraceValueBuilder(RawTraceValueHelpersMixin):
+  def __init__(self):
+    """TraceValueBuilder helps build up of a trace from multiple trace agents.
+
+    TraceValue is supposed to be immutable, but it is useful during recording
+    to have a mutable version. That is TraceValueBuilder.
+    """
+    self._raw_data = {}
+
+  def AsValue(self):
+    if self._raw_data == None:
+      raise Exception('Can only AsValue() once')
+
+    res = TraceValue()

[chrishenry, 2014/08/05 23:44:16]

nit: s/res/value/

+    res._SetFromBuilder(self._raw_data)
+    self._raw_data = None
+    return res
+
+  def AddEventsTo(self, part, events):
+    # Note: this wont work when you call this from multiple browsers.
+    # Each browser's trace_event_impl zeros its timestamps when it writes them
+    # out and doesn't write a timebase that can be used to re-sync them.

[chrishenry, 2014/08/05 23:44:16]

Turn this into pydoc?

+    assert isinstance(part, TraceValuePart)
+    assert isinstance(events, list)
+    if self._raw_data == None:
+      raise Exception('Already called AsValue() on this builder.')
+    existing_events = None
+    existing_events = self._raw_data.get(part.rawFieldName, None)
+    if existing_events == None:
+      existing_events = []
+      self._raw_data[part.rawFieldName] = existing_events
+    existing_events.extend(events)
+