{Csv,Block}PageMeasurementResults no longer perform streaming output.

As a results, kill now unused results_are_the_same_on_every_page.

This unfortunately changes the behaviors in the following ways:

1) Streaming Csv/Block results output on per page run (e.g., one
output per page repeat), the streaming mode merges like values for the
same page (in case of page repeat, it merges results across repeat).

2) Ordering of output rows may change.

BUG=346956,392901

Review URL: https://codereview.chromium.org/394953002

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

tools/perf/benchmarks/blink_perf.py

@@ -63,10 +63,6 @@ class _BlinkPerfMeasurement(page_measurement.PageMeasurement):
                            'blink_perf.js'), 'r') as f:
       self._blink_perf_js = f.read()
 
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False
-
   def WillNavigateToPage(self, page, tab):
     page.script_to_evaluate_on_commit = self._blink_perf_js
 

tools/perf/benchmarks/dom_perf.py

@@ -38,10 +38,6 @@ SCORE_TRACE_NAME = 'score'
 
 
 class _DomPerfMeasurement(page_measurement.PageMeasurement):
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False
-
   def MeasurePage(self, page, tab, results):
     try:
       def _IsDone():

tools/perf/measurements/loading_profile.py

@@ -17,10 +17,6 @@ class LoadingProfile(page_measurement.PageMeasurement):
   def __init__(self):
     super(LoadingProfile, self).__init__(discard_first_result=True)
 
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False
-
   def CustomizeBrowserOptions(self, options):
     if not perf_profiler.PerfProfiler.is_supported(browser_type='any'):
       raise Exception('This measurement is not supported on this platform')

tools/perf/measurements/loading_trace.py

@@ -12,10 +12,6 @@ class LoadingTrace(page_measurement.PageMeasurement):
     super(LoadingTrace, self).__init__(*args, **kwargs)
     self._timeline_controller = timeline_controller.TimelineController()
 
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False
-
   def WillNavigateToPage(self, page, tab):
     self._timeline_controller.Start(page, tab)
 

tools/perf/measurements/media.py

@@ -25,10 +25,6 @@ class Media(page_measurement.PageMeasurement):
     self._memory_metric = None
     self._power_metric = None
 
-  def results_are_the_same_on_every_page(self):
-    """Results can vary from page to page based on media events taking place."""
-    return False
-
   def WillStartBrowser(self, browser):
     self._power_metric = power.PowerMetric(browser)
 
@@ -70,4 +66,3 @@ class Media(page_measurement.PageMeasurement):
                                      trace_name=trace_name,
                                      exclude_metrics=exclude_metrics)
       self._power_metric.AddResults(tab, results)
-

tools/perf/measurements/page_cycler.py

@@ -171,6 +171,3 @@ class PageCycler(page_measurement.PageMeasurement):
     # warm run, and clearing the cache before the load of the following
     # URL would eliminate the intended warmup for the previous URL.
     return (self._has_loaded_page[url] >= self._cold_run_start_index)
-
-  def results_are_the_same_on_every_page(self):
-    return False

tools/perf/measurements/thread_times.py

@@ -18,10 +18,6 @@ class ThreadTimes(page_measurement.PageMeasurement):
     parser.add_option('--report-silk-details', action='store_true',
                       help='Report details relevant to silk.')
 
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False
-
   def WillRunActions(self, page, tab):
     self._timeline_controller = timeline_controller.TimelineController()
     if self.options.report_silk_details:

tools/telemetry/telemetry/page/page_measurement.py

@@ -63,16 +63,6 @@ class PageMeasurement(page_test.PageTest):
     finally:
       results.DidMeasurePage()
 
-  @property
-  def results_are_the_same_on_every_page(self):
-    """By default, measurements are assumed to output the same values for every
-    page. This allows incremental output, for example in CSV. If, however, the
-    measurement discovers what values it can report as it goes, and those values
-    may vary from page to page, you need to override this function and return
-    False. Output will not appear in this mode until the entire pageset has
-    run."""
-    return True
-
   def MeasurePage(self, page, tab, results):
     """Override to actually measure the page's performance.
 

tools/telemetry/telemetry/results/block_page_measurement_results.py

@@ -4,29 +4,33 @@
 import os
 
 from telemetry.results import page_measurement_results
+from telemetry.value import merge_values
 
 class BlockPageMeasurementResults(
     page_measurement_results.PageMeasurementResults):
   def __init__(self, output_stream):
     super(BlockPageMeasurementResults, self).__init__(output_stream)
 
-  def DidMeasurePage(self):
+  def PrintSummary(self):
     try:
-      values = self.page_specific_values_for_current_page
-      if not values:
-        # Do not output if no results were added on this page.
-        return
-      lines = ['name: %s' % values[0].page.display_name]
-      for value in sorted(values, key=lambda x: x.name):
-        if value.GetRepresentativeString() is not None:
+      values = merge_values.MergeLikeValuesFromSamePage(
+          self.all_page_specific_values)
+      value_groups_by_page = merge_values.GroupStably(
+          values, lambda value: value.page.url)
+      for values_for_page in value_groups_by_page:
+        if not values_for_page:
+          # Do not output if no results were added on this page.
+          return
+        lines = ['name: %s' % values_for_page[0].page.display_name]
+        for value in sorted(values_for_page, key=lambda x: x.name):
           lines.append('%s (%s): %s' %
                        (value.name,
                         value.units,
                         value.GetRepresentativeString()))
-      for line in lines:
-        self._output_stream.write(line)
+        for line in lines:
+          self._output_stream.write(line)
+          self._output_stream.write(os.linesep)
         self._output_stream.write(os.linesep)
-      self._output_stream.write(os.linesep)
-      self._output_stream.flush()
+        self._output_stream.flush()
     finally:
-      super(BlockPageMeasurementResults, self).DidMeasurePage()
+      super(BlockPageMeasurementResults, self).PrintSummary()

tools/telemetry/telemetry/results/block_page_measurement_results_unittest.py

@@ -41,7 +41,7 @@ class BlockPageMeasurementResultsTest(unittest.TestCase):
   def data(self):
     return [line.split(': ', 1) for line in self.lines]
 
-  def test_with_output_after_every_page(self):
+  def testOutput(self):
     results = NonPrintingBlockPageMeasurementResults(self._output)
     results.WillMeasurePage(self._page_set[0])
     results.AddValue(
@@ -53,12 +53,13 @@ class BlockPageMeasurementResultsTest(unittest.TestCase):
         scalar.ScalarValue(self._page_set[1], 'bar', 'seconds', 4))
     results.DidMeasurePage()
 
+    results.PrintSummary()
     expected = [
       ['name', 'http://www.foo.com/'],
-      ['foo (seconds)', '3'],
+      ['foo (seconds)', '[3]'],
       [''],
       ['name', 'http://www.bar.com/'],
-      ['bar (seconds)', '4'],
+      ['bar (seconds)', '[4]'],
       ['']
     ]
     self.assertEquals(self.data, expected)

tools/telemetry/telemetry/results/csv_page_measurement_results.py

 # Copyright 2014 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 csv
 
 from telemetry.results import page_measurement_results
@@ -9,36 +10,13 @@ from telemetry.value import merge_values
 
 class CsvPageMeasurementResults(
     page_measurement_results.PageMeasurementResults):
-  def __init__(self, output_stream, output_after_every_page=None):
+  def __init__(self, output_stream):
     super(CsvPageMeasurementResults, self).__init__(output_stream)
     self._results_writer = csv.writer(self._output_stream)
-    self._did_output_header = False
-    self._header_names_written_to_writer = None
-    self._output_after_every_page = output_after_every_page
-
-  def DidMeasurePage(self):
-    try:
-      assert self.page_specific_values_for_current_page != None
-
-      values = self.page_specific_values_for_current_page
-      if not values or not self._output_after_every_page:
-        # Do not output if no results were added on this page or if output flag
-        # is not set.
-        return
-
-      if not self._did_output_header:
-        self._OutputHeader(values)
-      else:
-        self._ValidateOutputNamesForCurrentPage()
-      self._OutputValuesForPage(values[0].page, values)
-    finally:
-      super(CsvPageMeasurementResults, self).DidMeasurePage()
+    self._representative_value_names = None
 
   def PrintSummary(self):
     try:
-      if self._output_after_every_page:
-        return
-
       values = merge_values.MergeLikeValuesFromSamePage(
           self.all_page_specific_values)
       self._OutputHeader(values)
@@ -49,18 +27,6 @@ class CsvPageMeasurementResults(
     finally:
       super(CsvPageMeasurementResults, self).PrintSummary()
 
-  def _ValidateOutputNamesForCurrentPage(self):
-    assert self._did_output_header
-    current_page_value_names = set([
-      value.name for value in self.page_specific_values_for_current_page])
-    header_names_written_to_writer = set(self._header_names_written_to_writer)
-    assert header_names_written_to_writer >= current_page_value_names, \
-        """To use CsvPageMeasurementResults, you must add the same
-result names for every page. In this case, the following extra results were
-added:
-%s
-""" % (current_page_value_names - header_names_written_to_writer)
-
   def _OutputHeader(self, values):
     """Output the header rows.
 
@@ -74,19 +40,17 @@ added:
       values: A set of values from which to extract the header string,
           which is the value name and the units.
     """
-    assert not self._did_output_header
-    representative_value_names = {}
+    representative_values = {}
     for value in values:
-      if value.name not in representative_value_names:
-        representative_value_names[value.name] = value
-    self._did_output_header = True
-    self._header_names_written_to_writer = list(
-        representative_value_names.keys())
-    self._header_names_written_to_writer.sort()
+      if value.name not in representative_values:
+        representative_values[value.name] = value
+    self._representative_value_names = list(
+        representative_values.keys())
+    self._representative_value_names.sort()
 
     row = ['page_name']
-    for value_name in self._header_names_written_to_writer:
-      units = representative_value_names[value_name].units
+    for value_name in self._representative_value_names:
+      units = representative_values[value_name].units
       row.append('%s (%s)' % (value_name, units))
     self._results_writer.writerow(row)
     self._output_stream.flush()
@@ -97,7 +61,7 @@ added:
     for value in page_values:
       values_by_value_name[value.name] = value
 
-    for value_name in self._header_names_written_to_writer:
+    for value_name in self._representative_value_names:
       value = values_by_value_name.get(value_name, None)
       if value and value.GetRepresentativeNumber():
         row.append('%s' % value.GetRepresentativeNumber())

tools/telemetry/telemetry/results/csv_page_measurement_results_unittest.py

@@ -46,30 +46,8 @@ class CsvPageMeasurementResultsTest(unittest.TestCase):
     rows = list(csv.reader(self.lines))
     return rows[1:]
 
-  def test_with_output_after_every_page(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, True)
-    results.WillMeasurePage(self._page_set[0])
-    results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
-    results.DidMeasurePage()
-    self.assertEquals(
-        self.output_header_row,
-        ['page_name', 'foo (seconds)'])
-    self.assertEquals(
-        self.output_data_rows[0],
-        [self._page_set[0].url, '3'])
-
-    results.WillMeasurePage(self._page_set[1])
-    results.AddValue(scalar.ScalarValue(self._page_set[1], 'foo', 'seconds', 4))
-    results.DidMeasurePage()
-    self.assertEquals(
-        len(self.output_data_rows),
-        2)
-    self.assertEquals(
-        self.output_data_rows[1],
-        [self._page_set[1].url, '4'])
-
   def test_with_no_results_on_second_run(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, True)
+    results = NonPrintingCsvPageMeasurementResults(self._output)
     results.WillMeasurePage(self._page_set[0])
     results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
     results.DidMeasurePage()
@@ -77,8 +55,15 @@ class CsvPageMeasurementResultsTest(unittest.TestCase):
     results.WillMeasurePage(self._page_set[1])
     results.DidMeasurePage()
 
+    results.PrintSummary()
+    self.assertEqual(['page_name', 'foo (seconds)'], self.output_header_row)
+    # TODO(chrishenry): Is this really the right behavior? Should this
+    # not output a second row with '-' as its results?
+    expected = [[self._page_set[0].url, '3.0']]
+    self.assertEqual(expected, self.output_data_rows)
+
   def test_fewer_results_on_second_run(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, True)
+    results = NonPrintingCsvPageMeasurementResults(self._output)
     results.WillMeasurePage(self._page_set[0])
     results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
     results.AddValue(scalar.ScalarValue(self._page_set[0], 'bar', 'seconds', 4))
@@ -88,36 +73,15 @@ class CsvPageMeasurementResultsTest(unittest.TestCase):
     results.AddValue(scalar.ScalarValue(self._page_set[1], 'bar', 'seconds', 5))
     results.DidMeasurePage()
 
-  def test_more_results_on_second_run(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, True)
-    results.WillMeasurePage(self._page_set[0])
-    results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
-    results.DidMeasurePage()
-
-    results.WillMeasurePage(self._page_set[1])
-    results.AddValue(scalar.ScalarValue(self._page_set[1], 'foo', 'seconds', 4))
-    results.AddValue(scalar.ScalarValue(self._page_set[1], 'bar', 'seconds', 5))
-
-    self.assertRaises(
-        Exception,
-        lambda: results.DidMeasurePage()) # pylint: disable=W0108
-
-  def test_with_output_after_every_page_and_inconsistency(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, True)
-    results.WillMeasurePage(self._page_set[0])
-    results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
-    results.DidMeasurePage()
-
-    # We printed foo, now change to bar
-    results.WillMeasurePage(self._page_set[1])
-    results.AddValue(scalar.ScalarValue(self._page_set[1], 'bar', 'seconds', 4))
-
-    self.assertRaises(
-        Exception,
-        lambda: results.DidMeasurePage()) # pylint: disable=W0108
+    results.PrintSummary()
+    self.assertEqual(['page_name', 'bar (seconds)', 'foo (seconds)'],
+                     self.output_header_row)
+    expected = [[self._page_set[0].url, '4.0', '3.0'],
+                [self._page_set[1].url, '5.0', '-']]
+    self.assertEqual(expected, self.output_data_rows)
 
   def test_with_output_at_print_summary_time(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, False)
+    results = NonPrintingCsvPageMeasurementResults(self._output)
     results.WillMeasurePage(self._page_set[0])
     results.AddValue(scalar.ScalarValue(self._page_set[0], 'foo', 'seconds', 3))
     results.DidMeasurePage()
@@ -128,16 +92,16 @@ class CsvPageMeasurementResultsTest(unittest.TestCase):
 
     results.PrintSummary()
 
-    self.assertEquals(
+    self.assertEqual(
       self.output_header_row,
       ['page_name', 'bar (seconds)', 'foo (seconds)'])
 
     expected = [[self._page_set[0].display_name, '-', '3.0'],
                 [self._page_set[1].display_name, '4.0', '-']]
-    self.assertEquals(expected, self.output_data_rows)
+    self.assertEqual(expected, self.output_data_rows)
 
   def test_histogram(self):
-    results = NonPrintingCsvPageMeasurementResults(self._output, False)
+    results = NonPrintingCsvPageMeasurementResults(self._output)
     results.WillMeasurePage(self._page_set[0])
     results.AddValue(histogram.HistogramValue(
         self._page_set[0], 'a', '',
@@ -152,10 +116,10 @@ class CsvPageMeasurementResultsTest(unittest.TestCase):
 
     results.PrintSummary()
 
-    self.assertEquals(
+    self.assertEqual(
         self.output_header_row,
         ['page_name', 'a ()'])
-    self.assertEquals(
+    self.assertEqual(
         self.output_data_rows,
         [[self._page_set[0].display_name, '1.5'],
          [self._page_set[1].display_name, '2.5']])

tools/telemetry/telemetry/results/results_options.py

@@ -68,10 +68,10 @@ def PrepareResults(test, options):
         output_stream, trace_tag=options.output_trace_tag)
   elif options.output_format == 'csv':
     return csv_page_measurement_results.CsvPageMeasurementResults(
-      output_stream, test.results_are_the_same_on_every_page)
+        output_stream)
   elif options.output_format == 'block':
     return block_page_measurement_results.BlockPageMeasurementResults(
-      output_stream)
+        output_stream)
   elif options.output_format == 'buildbot':
     return buildbot_page_measurement_results.BuildbotPageMeasurementResults(
         output_stream, trace_tag=options.output_trace_tag)

tools/telemetry/telemetry/web_perf/timeline_based_measurement.py

@@ -188,7 +188,3 @@ class TimelineBasedMeasurement(page_measurement.PageMeasurement):
   def CleanUpAfterPage(self, page, tab):
     if tab.browser.is_tracing_running:
       tab.browser.StopTracing()
-
-  @property
-  def results_are_the_same_on_every_page(self):
-    return False