Make MemoryMetric a subclass of Metric

BUG=

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

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

2 files changed, 48 insertions, 17 deletions

diff --git a/tools/perf/measurements/page_cycler.py b/tools/perf/measurements/page_cycler.py
index 967a67b..5ea98979 100644
--- a/tools/perf/measurements/page_cycler.py
+++ b/tools/perf/measurements/page_cycler.py
@@ -38,7 +38,7 @@ class PageCycler(page_measurement.PageMeasurement):
                            'page_cycler.js'), 'r') as f:
       self._page_cycler_js = f.read()
 
-    self._memory_metrics = None
+    self._memory_metric = None
     self._histograms = None
 
   def AddCommandLineOptions(self, parser):
@@ -51,9 +51,9 @@ class PageCycler(page_measurement.PageMeasurement):
     parser.add_option(pageset_repeat_option)
 
   def DidStartBrowser(self, browser):
-    self._memory_metrics = memory.MemoryMetrics()
-    self._memory_metrics.Start(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]
@@ -126,5 +126,7 @@ class PageCycler(page_measurement.PageMeasurement):
                 chart_name='times')
 
   def DidRunTest(self, tab, results):
-    self._memory_metrics.StopAndGetResults(tab.browser, results)
+    self._memory_metric.Stop()
+    self._memory_metric.AddResults(tab, results)
     self.MeasureIO(tab, results)
+
diff --git a/tools/perf/metrics/memory.py b/tools/perf/metrics/memory.py
index e918ac3..cbc0660 100644
--- a/tools/perf/metrics/memory.py
+++ b/tools/perf/metrics/memory.py
@@ -3,16 +3,30 @@
 # found in the LICENSE file.
 import sys
 
-class MemoryMetrics(object):
-  def __init__(self):
+from metrics import Metric
+
+class MemoryMetric(Metric):
+  """MemoryMetric gathers memory statistics from the browser object."""
+
+  def __init__(self, browser):
+    super(MemoryMetric, self).__init__()
+    self._browser = browser
+    self._memory_stats = None
     self._start_commit_charge = None
 
-  def Start(self, browser):
-    self._start_commit_charge = browser.memory_stats['SystemCommitCharge']
+  def Start(self, page=None, tab=None):
+    """Record the initial value of 'SystemCommitCharge'."""
+    self._start_commit_charge = self._browser.memory_stats['SystemCommitCharge']
+
+  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
 
-  def StopAndGetResults(self, browser, results):
-    memory = browser.memory_stats
-    if not memory['Browser']:
+  def AddResults(self, tab, results):
+    """Add summary results to the results object."""
+    assert self._memory_stats, 'Must call Stop() first'
+    if not self._memory_stats['Browser']:
       return
 
     metric = 'resident_set_size'
@@ -20,16 +34,30 @@ class MemoryMetrics(object):
       metric = 'working_set'
 
     def AddSummariesForProcessTypes(process_types_memory, process_type_trace):
+      """Add all summaries to the results for a given set of process types.
+
+      Args:
+        process_types_memory: A list of process types, e.g. Browser, 'Renderer'
+        process_type_trace: The name of this set of process types in the output
+      """
       def AddSummary(value_name_memory, value_name_trace):
+        """Add a summary to the results for a given statistic.
+
+        Args:
+          value_name_memory: Name of some statistic, e.g. VM, WorkingSetSize
+          value_name_trace: Name of this statistic to be used in the output
+        """
         if len(process_types_memory) > 1 and value_name_memory.endswith('Peak'):
           return
         values = []
         for process_type_memory in process_types_memory:
-          if value_name_memory in memory[process_type_memory]:
-            values.append(memory[process_type_memory][value_name_memory])
+          stats = self._memory_stats[process_type_memory]
+          if value_name_memory in stats:
+            values.append(stats[value_name_memory])
         if values:
           results.AddSummary(value_name_trace + process_type_trace,
                              'bytes', sum(values), data_type='unimportant')
+
       AddSummary('VM', 'vm_final_size_')
       AddSummary('WorkingSetSize', 'vm_%s_final_size_' % metric)
       AddSummary('PrivateDirty', 'vm_private_dirty_final_')
@@ -42,9 +70,10 @@ class MemoryMetrics(object):
     AddSummariesForProcessTypes(['Gpu'], 'gpu')
     AddSummariesForProcessTypes(['Browser', 'Renderer', 'Gpu'], 'total')
 
-    results.AddSummary('commit_charge', 'kb',
-                       memory['SystemCommitCharge'] - self._start_commit_charge,
+    end_commit_charge = self._memory_stats['SystemCommitCharge']
+    commit_charge_difference = end_commit_charge - self._start_commit_charge
+    results.AddSummary('commit_charge', 'kb', commit_charge_difference,
                        data_type='unimportant')
-    results.AddSummary('processes', 'count', memory['ProcessCount'],
+    results.AddSummary('processes', 'count', self._memory_stats['ProcessCount'],
                        data_type='unimportant')