Make page_cycler.py fully measure memory for each page.

tools/perf/metrics/memory.py

 # 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 = [
@@ skipping 68 matching lines @@
 
   def AddResults(self, tab, results):
     """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, trace_name=None):
-    """Add summary (overall) results to the results object."""
     self._memory_stats = self._browser.memory_stats
     if not self._memory_stats['Browser']:
       return
 
     metric = 'resident_set_size'
     if sys.platform == 'win32':
       metric = 'working_set'
 
-    def AddSummariesForProcessTypes(process_types_memory, process_type_trace):
-      """Add all summaries to the results for a given set of process types.
+    def AddResultsForProcessTypes(process_types_memory, process_type_trace):
+      """Add all 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.
+      def AddResult(value_name_memory, value_name_trace):
+        """Add a result 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:
           stats = self._memory_stats[process_type_memory]
           if value_name_memory in stats:
             values.append(stats[value_name_memory])
         if values:
-          if trace_name:

[shadi, 2013/12/02 23:09:02]

Why removing all 'trace_name' support? This will break existing traces and
create bunch of new ones for media benchmark at least.

[Philippe, 2013/12/03 12:55:00]

Should I keep the AddSummaryResults() method and factor out its body (that would
be used by AddResults() too) so that media benchmarks are not affected? For page
cyclers we would only use AddResults(). I'm not familiar with media benchmarks
so any guidance would be appreciated :)

-            current_trace = '%s_%s' % (trace_name, process_type_trace)
-            chart_name = value_name_trace
-          else:
-            current_trace = '%s_%s' % (value_name_trace, process_type_trace)
-            chart_name = current_trace
-          results.AddSummary(current_trace, 'bytes', sum(values),
-                             chart_name=chart_name, data_type='unimportant')
+          current_trace = '%s_%s' % (value_name_trace, process_type_trace)
+          chart_name = current_trace
+          results.Add(current_trace, 'bytes', sum(values),
+                      chart_name=chart_name, data_type='unimportant')
 
-      AddSummary('VM', 'vm_final_size')
-      AddSummary('WorkingSetSize', 'vm_%s_final_size' % metric)
-      AddSummary('PrivateDirty', 'vm_private_dirty_final')
-      AddSummary('ProportionalSetSize', 'vm_proportional_set_size_final')
-      AddSummary('SharedDirty', 'vm_shared_dirty_final')
-      AddSummary('VMPeak', 'vm_peak_size')
-      AddSummary('WorkingSetSizePeak', '%s_peak_size' % metric)
+      AddResult('VM', 'vm_final_size')
+      AddResult('WorkingSetSize', 'vm_%s_final_size' % metric)
+      AddResult('PrivateDirty', 'vm_private_dirty_final')
+      AddResult('ProportionalSetSize', 'vm_proportional_set_size_final')
+      AddResult('SharedDirty', 'vm_shared_dirty_final')
+      AddResult('VMPeak', 'vm_peak_size')
+      AddResult('WorkingSetSizePeak', '%s_peak_size' % metric)
 
-    AddSummariesForProcessTypes(['Browser'], 'browser')
-    AddSummariesForProcessTypes(['Renderer'], 'renderer')
-    AddSummariesForProcessTypes(['Gpu'], 'gpu')
-    AddSummariesForProcessTypes(['Browser', 'Renderer', 'Gpu'], 'total')
+    AddResultsForProcessTypes(['Browser'], 'browser')
+    AddResultsForProcessTypes(['Renderer'], 'renderer')
+    AddResultsForProcessTypes(['Gpu'], 'gpu')
+    AddResultsForProcessTypes(['Browser', 'Renderer', 'Gpu'], 'total')
 
     end_commit_charge = self._memory_stats['SystemCommitCharge']
     commit_charge_difference = end_commit_charge - self._start_commit_charge
-    results.AddSummary(trace_name or 'commit_charge', 'kb',
-                       commit_charge_difference,
-                       chart_name='commit_charge',
-                       data_type='unimportant')
-    results.AddSummary(trace_name or 'processes', 'count',
-                       self._memory_stats['ProcessCount'],
-                       chart_name='processes',
-                       data_type='unimportant')
+    results.Add('commit_charge', 'kb',
+                commit_charge_difference,
+                chart_name='commit_charge',
+                data_type='unimportant')
+    results.Add('processes', 'count',
+                self._memory_stats['ProcessCount'],
+                chart_name='processes',
+                data_type='unimportant')
 
+  def AddSummaryResults(self, results, trace_name=None):

[tonyg, 2013/11/27 17:59:58]

I think this can just be omitted, right?

[qyearsley, 2013/11/27 19:01:55]

In general, Metrics don't need to have a method AddSummaryResults, since it
isn't in the Metric base class right now (although it could be put in). Once the
call to this method is removed from the memory, memory_multi_tab and media
measurements, this can be removed.

[Philippe, 2013/11/28 10:30:13]

Yeah, I removed this method.

+    # This metric only returns per-page results.
+    raise NotImplementedError()