Move memory-related histogram metrics into metrics/memory.py

BUG=

Review URL: https://chromiumcodereview.appspot.com/22934009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@217911 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 112 insertions, 117 deletions

diff --git a/tools/perf/measurements/memory.py b/tools/perf/measurements/memory.py
index f15184a..6aa88be 100644
--- a/tools/perf/measurements/memory.py
+++ b/tools/perf/measurements/memory.py
@@ -1,42 +1,20 @@
 # Copyright (c) 2012 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.
-from metrics import histogram
+
 from metrics import memory
 from telemetry.page import page_measurement
 
-
-MEMORY_HISTOGRAMS = [
-    {'name': 'V8.MemoryExternalFragmentationTotal', 'units': 'percent'},
-    {'name': 'V8.MemoryHeapSampleTotalCommitted', 'units': 'kb'},
-    {'name': 'V8.MemoryHeapSampleTotalUsed', 'units': 'kb'},
-    {'name': 'Memory.RendererUsed', 'units': 'kb'}]
-
-
-BROWSER_MEMORY_HISTOGRAMS =  [
-    {'name': 'Memory.BrowserUsed', 'units': 'kb'}]
-
-
 class Memory(page_measurement.PageMeasurement):
   def __init__(self):
     super(Memory, self).__init__('stress_memory')
-    self.histograms = (
-        [histogram.HistogramMetric(
-            h, histogram.RENDERER_HISTOGRAM)
-         for h in MEMORY_HISTOGRAMS] +
-        [histogram.HistogramMetric(
-            h, histogram.BROWSER_HISTOGRAM)
-         for h in BROWSER_MEMORY_HISTOGRAMS])
-
     self._memory_metric = None
 
   def DidStartBrowser(self, browser):
     self._memory_metric = memory.MemoryMetric(browser)
-    self._memory_metric.Start()
 
   def DidNavigateToPage(self, page, tab):
-    for h in self.histograms:
-      h.Start(page, tab)
+    self._memory_metric.Start(page, tab)
 
   def CustomizeBrowserOptions(self, options):
     options.AppendExtraBrowserArg('--enable-stats-collection-bindings')
@@ -59,8 +37,8 @@ class Memory(page_measurement.PageMeasurement):
     return hasattr(page, 'stress_memory')
 
   def MeasurePage(self, page, tab, results):
-    for h in self.histograms:
-      h.GetValue(page, tab, results)
+    self._memory_metric.Stop(page, tab)
+    self._memory_metric.AddResults(tab, results)
 
     if tab.browser.is_profiler_active('tcmalloc-heap'):
       # The tcmalloc_heap_profiler dumps files at regular
@@ -75,6 +53,5 @@ class Memory(page_measurement.PageMeasurement):
       """)
 
   def DidRunTest(self, tab, results):
-    self._memory_metric.Stop()
-    self._memory_metric.AddResults(tab, results)
+    self._memory_metric.AddSummaryResults(results)
 
diff --git a/tools/perf/measurements/page_cycler.py b/tools/perf/measurements/page_cycler.py
index 50d11e8..f4d3cf2 100644
--- a/tools/perf/measurements/page_cycler.py
+++ b/tools/perf/measurements/page_cycler.py
@@ -18,19 +18,11 @@ cycling all pages.
 import os
 import sys
 
-from metrics import histogram
 from metrics import io
 from metrics import memory
 from telemetry.core import util
 from telemetry.page import page_measurement
 
-
-MEMORY_HISTOGRAMS = [
-    {'name': 'V8.MemoryExternalFragmentationTotal', 'units': 'percent'},
-    {'name': 'V8.MemoryHeapSampleTotalCommitted', 'units': 'kb'},
-    {'name': 'V8.MemoryHeapSampleTotalUsed', 'units': 'kb'}]
-
-
 class PageCycler(page_measurement.PageMeasurement):
   def __init__(self, *args, **kwargs):
     super(PageCycler, self).__init__(*args, **kwargs)
@@ -40,7 +32,6 @@ class PageCycler(page_measurement.PageMeasurement):
       self._page_cycler_js = f.read()
 
     self._memory_metric = None
-    self._histograms = None
 
   def AddCommandLineOptions(self, parser):
     # The page cyclers should default to 10 iterations. In order to change the
@@ -54,10 +45,6 @@ class PageCycler(page_measurement.PageMeasurement):
   def DidStartBrowser(self, browser):
     """Initialize metrics once right after the browser has been launched."""
     self._memory_metric = memory.MemoryMetric(browser)
-    self._memory_metric.Start()
-    self._histograms = [histogram.HistogramMetric(
-                           h, histogram.RENDERER_HISTOGRAM)
-                       for h in MEMORY_HISTOGRAMS]
 
   def DidStartHTTPServer(self, tab):
     # Avoid paying for a cross-renderer navigation on the first page on legacy
@@ -68,8 +55,7 @@ class PageCycler(page_measurement.PageMeasurement):
     page.script_to_evaluate_on_commit = self._page_cycler_js
 
   def DidNavigateToPage(self, page, tab):
-    for h in self._histograms:
-      h.Start(page, tab)
+    self._memory_metric.Start(page, tab)
 
   def CustomizeBrowserOptions(self, options):
     options.AppendExtraBrowserArg('--enable-stats-collection-bindings')
@@ -84,21 +70,18 @@ class PageCycler(page_measurement.PageMeasurement):
       print 'typical_25 is currently disabled on mac. Skipping test.'
       sys.exit(0)
 
-
   def MeasurePage(self, page, tab, results):
     def _IsDone():
       return bool(tab.EvaluateJavaScript('__pc_load_time'))
     util.WaitFor(_IsDone, 60)
-
-    for h in self._histograms:
-      h.GetValue(page, tab, results)
-
     results.Add('page_load_time', 'ms',
                 int(float(tab.EvaluateJavaScript('__pc_load_time'))),
                 chart_name='times')
 
-  def DidRunTest(self, tab, results):
-    self._memory_metric.Stop()
+    self._memory_metric.Stop(page, tab)
     self._memory_metric.AddResults(tab, results)
+
+  def DidRunTest(self, tab, results):
+    self._memory_metric.AddSummaryResults(results)
     io.IOMetric().AddSummaryResults(tab, results)
 
diff --git a/tools/perf/measurements/tab_switching.py b/tools/perf/measurements/tab_switching.py
index b5b6c1f..fc083e1 100644
--- a/tools/perf/measurements/tab_switching.py
+++ b/tools/perf/measurements/tab_switching.py
@@ -38,7 +38,7 @@ class TabSwitching(page_measurement.PageMeasurement):
     thenrecord a single histogram for the tab switching metric.
     """
     histogram_name = 'MPArch.RWH_TabSwitchPaintDuration'
-    histogram_type = 'getBrowserHistogram'
+    histogram_type = histogram_util.BROWSER_HISTOGRAM
     first_histogram = histogram_util.GetHistogramFromDomAutomation(
         histogram_type, histogram_name, tab)
     prev_histogram = first_histogram
diff --git a/tools/perf/metrics/histogram.py b/tools/perf/metrics/histogram.py
deleted file mode 100644
index 147ae96..0000000
--- a/tools/perf/metrics/histogram.py
+++ /dev/null
@@ -1,40 +0,0 @@
-# Copyright 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.
-from metrics import histogram_util
-
-BROWSER_HISTOGRAM = 'browser_histogram'
-RENDERER_HISTOGRAM = 'renderer_histogram'
-
-class HistogramMetric(object):
-  def __init__(self, histogram, histogram_type):
-    self.name = histogram['name']
-    self.units = histogram['units']
-    self.histogram_type = histogram_type
-    self._start_values = dict()
-
-  def Start(self, page, tab):
-    """Get the starting value for the histogram. This value will then be
-    subtracted from the actual measurement."""
-    data = self._GetHistogramFromDomAutomation(tab)
-    if data:
-      self._start_values[page.url + self.name] = data
-
-  def GetValue(self, page, tab, results):
-    data = self._GetHistogramFromDomAutomation(tab)
-    if not data:
-      return
-    new_histogram = histogram_util.SubtractHistogram(
-        data, self._start_values[page.url + self.name])
-    results.Add(self.name, self.units, new_histogram,
-                data_type='unimportant-histogram')
-
-  @property
-  def histogram_function(self):
-    if self.histogram_type == BROWSER_HISTOGRAM:
-      return 'getBrowserHistogram'
-    return 'getHistogram'
-
-  def _GetHistogramFromDomAutomation(self, tab):
-    return histogram_util.GetHistogramFromDomAutomation(
-        self.histogram_function, self.name, tab)
diff --git a/tools/perf/metrics/histogram_util.py b/tools/perf/metrics/histogram_util.py
index 5b72f5e..1f255ca 100644
--- a/tools/perf/metrics/histogram_util.py
+++ b/tools/perf/metrics/histogram_util.py
@@ -1,12 +1,27 @@
 # Copyright 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.
+
+"""This is a helper module to get and manipulate histogram data.
+
+The histogram data is the same data as is visible from "chrome://histograms".
+More information can be found at: chromium/src/base/metrics/histogram.h
+
+Histogram data is collected with either the window.statsCollectionController
+object or the window.domAutomationController object.
+"""
+
 import json
 import logging
 
+BROWSER_HISTOGRAM = 'browser_histogram'
+RENDERER_HISTOGRAM = 'renderer_histogram'
+
 def SubtractHistogram(histogram_json, start_histogram_json):
-  """Subtracts a previous histogram from a histogram. Both parameters are json
-  serializations of histograms."""
+  """Subtracts a previous histogram from a histogram.
+
+  Both parameters and the returned result are json serializations.
+  """
   start_histogram = json.loads(start_histogram_json)
   # It's ok if the start histogram is empty (we had no data, maybe even no
   # histogram at all, at the start of the test).
@@ -40,11 +55,20 @@ def SubtractHistogram(histogram_json, start_histogram_json):
 
   return json.dumps(histogram)
 
-def GetHistogramFromDomAutomation(function, name, tab):
+
+def GetHistogramFromDomAutomation(histogram_type, histogram_name, tab):
+  """Get a json serialization of a histogram."""
+  assert histogram_type in [BROWSER_HISTOGRAM, RENDERER_HISTOGRAM]
+  function = 'getHistogram'
+  if histogram_type == BROWSER_HISTOGRAM:
+    function = 'getBrowserHistogram'
   # TODO(jeremy): Remove references to
   # domAutomationController when we update the reference builds.
-  js = ('(window.statsCollectionController ? '
-        'statsCollectionController : '
-        'domAutomationController).%s("%s")' %
-        (function, name))
-  return tab.EvaluateJavaScript(js)
+  histogram_json = tab.EvaluateJavaScript(
+      '(window.statsCollectionController ? '
+      'statsCollectionController : '
+      'domAutomationController).%s("%s")' %
+      (function, histogram_name))
+  if histogram_json:
+    return histogram_json
+  return None
diff --git a/tools/perf/metrics/histogram_util_unittest.py b/tools/perf/metrics/histogram_util_unittest.py
index 7db5558..7cf313a 100644
--- a/tools/perf/metrics/histogram_util_unittest.py
+++ b/tools/perf/metrics/histogram_util_unittest.py
@@ -1,6 +1,7 @@
 # Copyright 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 json
 import unittest
 
@@ -9,16 +10,16 @@ from metrics import histogram_util
 class TestHistogram(unittest.TestCase):
   def testSubtractHistogram(self):
     baseline_histogram = """{"count": 3, "buckets": [
-{"low": 1, "high": 2, "count": 1},
-{"low": 2, "high": 3, "count": 2}]}"""
+        {"low": 1, "high": 2, "count": 1},
+        {"low": 2, "high": 3, "count": 2}]}"""
 
-    histogram = """{"count": 14, "buckets": [
-{"low": 1, "high": 2, "count": 1},
-{"low": 2, "high": 3, "count": 3},
-{"low": 3, "high": 4, "count": 10}]}"""
+    later_histogram = """{"count": 14, "buckets": [
+        {"low": 1, "high": 2, "count": 1},
+        {"low": 2, "high": 3, "count": 3},
+        {"low": 3, "high": 4, "count": 10}]}"""
 
     new_histogram = json.loads(
-        histogram_util.SubtractHistogram(histogram, baseline_histogram))
+        histogram_util.SubtractHistogram(later_histogram, baseline_histogram))
     new_buckets = dict()
     for b in new_histogram['buckets']:
       new_buckets[b['low']] = b['count']
diff --git a/tools/perf/metrics/memory.py b/tools/perf/metrics/memory.py
index cbc0660..3a72434 100644
--- a/tools/perf/metrics/memory.py
+++ b/tools/perf/metrics/memory.py
@@ -1,31 +1,81 @@
 # Copyright 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 sys
 
+from metrics import histogram_util
 from metrics import Metric
 
+_HISTOGRAMS = [
+    {'name': 'V8.MemoryExternalFragmentationTotal', 'units': 'percent',
+     'type': histogram_util.RENDERER_HISTOGRAM},
+    {'name': 'V8.MemoryHeapSampleTotalCommitted', 'units': 'kb',
+     'type': histogram_util.RENDERER_HISTOGRAM},
+    {'name': 'V8.MemoryHeapSampleTotalUsed', 'units': 'kb',
+     'type': histogram_util.RENDERER_HISTOGRAM},
+    {'name': 'Memory.RendererUsed', 'units': 'kb',
+     'type': histogram_util.RENDERER_HISTOGRAM},
+    {'name': 'Memory.BrowserUsed', 'units': 'kb',
+     'type': histogram_util.BROWSER_HISTOGRAM}]
+
 class MemoryMetric(Metric):
-  """MemoryMetric gathers memory statistics from the browser object."""
+  """MemoryMetric gathers memory statistics from the browser object.
+
+  This includes both per-page histogram stats, most about javascript
+  memory usage, and overall memory stats from the system for the whole
+  test run."""
 
   def __init__(self, browser):
     super(MemoryMetric, self).__init__()
     self._browser = browser
+    self._start_commit_charge = self._browser.memory_stats['SystemCommitCharge']
     self._memory_stats = None
-    self._start_commit_charge = None
+    self._histogram_start = dict()
+    self._histogram_delta = dict()
 
-  def Start(self, page=None, tab=None):
-    """Record the initial value of 'SystemCommitCharge'."""
-    self._start_commit_charge = self._browser.memory_stats['SystemCommitCharge']
+  def Start(self, page, tab):
+    """Start the per-page preparation for this metric.
 
-  def Stop(self, page=None, tab=None):
-    """Fetch the browser memory stats."""
-    assert self._start_commit_charge, 'Must call Start() first'
-    self._memory_stats = self._browser.memory_stats
+    Here, this consists of recording the start value of all the histograms.
+    """
+    for h in _HISTOGRAMS:
+      histogram_data = histogram_util.GetHistogramFromDomAutomation(
+          h['type'], h['name'], tab)
+      # Histogram data may not be available
+      if not histogram_data:
+        continue
+      self._histogram_start[h['name']] = histogram_data
+
+  def Stop(self, page, tab):
+    """Prepare the results for this page.
+
+    The results are the differences between the current histogram values
+    and the values when Start() was called.
+    """
+    assert self._histogram_start, 'Must call Start() first'
+    for h in _HISTOGRAMS:
+      # Histogram data may not be available
+      if h['name'] not in self._histogram_start:
+        continue
+      histogram_data = histogram_util.GetHistogramFromDomAutomation(
+          h['type'], h['name'], tab)
+      self._histogram_delta[h['name']] = histogram_util.SubtractHistogram(
+          histogram_data, self._histogram_start[h['name']])
 
   def AddResults(self, tab, results):
-    """Add summary results to the results object."""
-    assert self._memory_stats, 'Must call Stop() first'
+    """Add results for this page to the results object."""
+    assert self._histogram_delta, 'Must call Stop() first'
+    for h in _HISTOGRAMS:
+      # Histogram data may not be available
+      if h['name'] not in self._histogram_start:
+        continue
+      results.Add(h['name'], h['units'], self._histogram_delta[h['name']],
+                  data_type='unimportant-histogram')
+
+  def AddSummaryResults(self, results):
+    """Add summary (overall) results to the results object."""
+    self._memory_stats = self._browser.memory_stats
     if not self._memory_stats['Browser']:
       return