Move memory-related histogram data collection to metrics/memory.py

tools/perf/measurements/page_cycler.py

Index: tools/perf/measurements/page_cycler.py
diff --git a/tools/perf/measurements/page_cycler.py b/tools/perf/measurements/page_cycler.py
index 5ea989796bd2b004bf75a62ca6007f4d016e5bf4..143b02a198d89500a5ecebaaadc99d2d00326359 100644
--- a/tools/perf/measurements/page_cycler.py
+++ b/tools/perf/measurements/page_cycler.py
@@ -18,18 +18,10 @@ cycling all pages.
 import os
 import sys
 
-from metrics import histogram
 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)
@@ -39,7 +31,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
@@ -53,10 +44,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
@@ -67,8 +54,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')
@@ -114,19 +100,17 @@ class PageCycler(page_measurement.PageMeasurement):
     AddSummariesForProcessType('Gpu', 'gpu')
 
   def MeasurePage(self, page, tab, results):
+    self._memory_metric.Stop(page, tab)
+    self._memory_metric.AddResults(tab, results)

[tonyg, 2013/08/09 01:43:44]

I think these need to be after we wait for the page to load, right?

[qyearsley, 2013/08/09 16:43:15]

Yeah -- that's how it has been, so if want to keep the behavior the same as
before, then that's how it should be.

However, if we wanted to change it to be exactly the same as the memory
measurement, then this could be kept at the top of MeasurePage. Do we want to
make the memory-related results for page cycler measurement and memory
measurement exactly the same?

(I think that the waiting for load is mostly important for the purposes of the
'page_load_time' result... which incidentally will soon be replaced by the speed
index metric. And the speed index metric might wait even longer... perhaps when
that happens, we still want the memory-related to results to be about the state
of memory when MeaurePage is called?)

+
     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.AddResults(tab, results)
+    self._memory_metric.AddSummaryResults(results)
     self.MeasureIO(tab, results)