Make page_cycler.py fully measure memory for each page.

tools/perf/metrics/memory.py

Index: tools/perf/metrics/memory.py
diff --git a/tools/perf/metrics/memory.py b/tools/perf/metrics/memory.py
index a59027c41a060aecb72bdfc5e23339451f68481c..c38d9961f4a2a77e6e15e628b30b0d535725a018 100644
--- a/tools/perf/metrics/memory.py
+++ b/tools/perf/metrics/memory.py
@@ -86,9 +86,6 @@ class MemoryMetric(Metric):
         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
@@ -97,15 +94,15 @@ class MemoryMetric(Metric):
     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
@@ -119,36 +116,35 @@ class MemoryMetric(Metric):
           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')
-
-      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)
-
-    AddSummariesForProcessTypes(['Browser'], 'browser')
-    AddSummariesForProcessTypes(['Renderer'], 'renderer')
-    AddSummariesForProcessTypes(['Gpu'], 'gpu')
-    AddSummariesForProcessTypes(['Browser', 'Renderer', 'Gpu'], 'total')
+          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')
+
+      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)
+
+    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()