[bisect] - First pass bisect functional breakages.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 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.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 159 matching lines @@
 # once build is produced, it reads SHA value from this file and appends it
 # to build archive filename.
 DEPS_SHA_PATCH = """diff --git src/DEPS.sha src/DEPS.sha
 new file mode 100644
 --- /dev/null
 +++ src/DEPS.sha
 @@ -0,0 +1 @@
 +%(deps_sha)s
 """
 
+BISECT_MODE_MEAN = 'mean'
+BISECT_MODE_STD_DEV = 'std_dev'
+BISECT_MODE_RETURN_CODE = 'return_code'

[qyearsley, 2014/04/25 23:53:29]

Could add a comment about what these three constants are used for: they
represent the three possible values of of the --bisect-mode flag, which
determines what value is used to classify a revision as "good" or "bad", right?

+
+
 def _AddAdditionalDepotInfo(depot_info):
   """Adds additional depot info to the global depot variables."""
   global DEPOT_DEPS_NAME
   global DEPOT_NAMES
   DEPOT_DEPS_NAME = dict(DEPOT_DEPS_NAME.items() +
       depot_info.items())
   DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
 
 
 def CalculateTruncatedMean(data_set, truncate_percent):
@@ skipping 1771 matching lines @@
       path_to_generate = os.path.join('tools', 'perf', 'generate_profile')
 
       if arg_dict.has_key('--profile-dir') and arg_dict.has_key('--browser'):
         profile_path, profile_type = os.path.split(arg_dict['--profile-dir'])
         return not RunProcess(['python', path_to_generate,
             '--profile-type-to-generate', profile_type,
             '--browser', arg_dict['--browser'], '--output-dir', profile_path])
       return False
     return True
 
+  def _IsBisectModeUsingMetric(self):
+    return self.opts.bisect_mode in [BISECT_MODE_MEAN, BISECT_MODE_STD_DEV]
+
+  def _IsBisectModeReturnCode(self):
+    return self.opts.bisect_mode in [BISECT_MODE_RETURN_CODE]
+
+  def _IsBisectModeStandardDeviation(self):
+    return self.opts.bisect_mode in [BISECT_MODE_STD_DEV]
+
   def RunPerformanceTestAndParseResults(
       self, command_to_run, metric, reset_on_first_run=False,
       upload_on_last_run=False, results_label=None):
     """Runs a performance test on the current revision and parses the results.
 
     Args:
       command_to_run: The command to be run to execute the performance test.
       metric: The metric to parse out from the results of the performance test.
           This is the result chart name and trace name, separated by slash.
       reset_on_first_run: If True, pass the flag --reset-results on first run.
@@ skipping 34 matching lines @@
     if self.opts.target_platform == 'cros' and is_telemetry:
       args.append('--remote=%s' % self.opts.cros_remote_ip)
       args.append('--identity=%s' % CROS_TEST_KEY_PATH)
 
     start_time = time.time()
 
     metric_values = []
     output_of_all_runs = ''
     for i in xrange(self.opts.repeat_test_count):
       # Can ignore the return code since if the tests fail, it won't return 0.
+      current_args = copy.copy(args)
+      if is_telemetry:
+        if i == 0 and reset_on_first_run:
+          current_args.append('--reset-results')
+        elif i == self.opts.repeat_test_count - 1 and upload_on_last_run:
+          current_args.append('--upload-results')
+        if results_label:
+          current_args.append('--results-label=%s' % results_label)
       try:
-        current_args = copy.copy(args)
-        if is_telemetry:
-          if i == 0 and reset_on_first_run:
-            current_args.append('--reset-results')
-          elif i == self.opts.repeat_test_count - 1 and upload_on_last_run:
-            current_args.append('--upload-results')
-          if results_label:
-            current_args.append('--results-label=%s' % results_label)
         (output, return_code) = RunProcessAndRetrieveOutput(current_args,
                                                             cwd=self.src_cwd)
       except OSError, e:
         if e.errno == errno.ENOENT:
           err_text  = ('Something went wrong running the performance test. '
                        'Please review the command line:\n\n')
           if 'src/' in ' '.join(args):
             err_text += ('Check that you haven\'t accidentally specified a '
                          'path with src/ in the command.\n\n')
           err_text += ' '.join(args)
           err_text += '\n'
 
           return (err_text, failure_code)
         raise
 
       output_of_all_runs += output
       if self.opts.output_buildbot_annotations:
         print output
 
-      metric_values += self.ParseMetricValuesFromOutput(metric, output)
+      if self._IsBisectModeUsingMetric():
+        metric_values += self.ParseMetricValuesFromOutput(metric, output)
+        # If we're bisecting on a metric (ie, changes in the mean or
+        # standard deviation) and no metric values are produced, bail out.
+        if not metric_values:
+          break
+      elif self._IsBisectModeReturnCode():
+        metric_values.append(return_code)
 
       elapsed_minutes = (time.time() - start_time) / 60.0
-
-      if elapsed_minutes >= self.opts.max_time_minutes or not metric_values:
+      if elapsed_minutes >= self.opts.max_time_minutes:
         break
 
     if len(metric_values) == 0:
       err_text = 'Metric %s was not found in the test output.' % metric
       # TODO(qyearsley): Consider also getting and displaying a list of metrics
       # that were found in the output here.
       return (err_text, failure_code, output_of_all_runs)
 
-    # Need to get the average value if there were multiple values.
-    truncated_mean = CalculateTruncatedMean(metric_values,
-        self.opts.truncate_percent)
-    standard_err = CalculateStandardError(metric_values)
-    standard_dev = CalculateStandardDeviation(metric_values)
+    # If we're bisecting on return codes, we're really just looking for zero vs
+    # non-zero.
+    if self._IsBisectModeReturnCode():
+      # If any of the return codes is non-zero, output 1.
+      overall_return_code = 0 if (
+          all(current_value == 0 for current_value in metric_values)) else 1
 
-    values = {
-      'mean': truncated_mean,
-      'std_err': standard_err,
-      'std_dev': standard_dev,
-      'values': metric_values,
-    }
+      values = {
+        'mean': overall_return_code,
+        'std_err': 0.0,
+        'std_dev': 0.0,
+        'values': metric_values,
+      }

[qyearsley, 2014/04/25 23:53:29]

It's potentially confusing that "mean" could be overall_return_code, since
that's not actually a mean, and also there no standard deviation or standard
error were actually calculated.

However, if you change this values dict to contain different key names, then
you'll have to change other things elsewhere, which would make this CL larger.

 
-    print 'Results of performance test: %12f %12f' % (
-        truncated_mean, standard_err)
-    print
+      print 'Results of performance test: Command returned with %d' % (
+          overall_return_code)
+      print
+    else:
+      # Need to get the average value if there were multiple values.
+      truncated_mean = CalculateTruncatedMean(metric_values,
+          self.opts.truncate_percent)
+      standard_err = CalculateStandardError(metric_values)
+      standard_dev = CalculateStandardDeviation(metric_values)
+
+      if self._IsBisectModeStandardDeviation():
+        metric_values = [standard_dev]
+
+      values = {
+        'mean': truncated_mean,
+        'std_err': standard_err,
+        'std_dev': standard_dev,
+        'values': metric_values,
+      }
+
+      print 'Results of performance test: %12f %12f' % (
+          truncated_mean, standard_err)
+      print
     return (values, success_code, output_of_all_runs)
 
   def FindAllRevisionsToSync(self, revision, depot):
     """Finds all dependant revisions and depots that need to be synced for a
     given revision. This is only useful in the git workflow, as an svn depot
     may be split into multiple mirrors.
 
     ie. skia is broken up into 3 git mirrors over skia/src, skia/gyp, and
     skia/include. To sync skia/src properly, one has to find the proper
     revisions in skia/gyp and skia/include.
@@ skipping 240 matching lines @@
             return results
         else:
           return ('Failed to build revision: [%s]' % (str(revision, )),
               BUILD_RESULT_FAIL)
       else:
         return ('Failed to run [gclient runhooks].', BUILD_RESULT_FAIL)
     else:
       return ('Failed to sync revision: [%s]' % (str(revision, )),
           BUILD_RESULT_FAIL)
 
-  def CheckIfRunPassed(self, current_value, known_good_value, known_bad_value):
+  def _CheckIfRunPassed(self, current_value, known_good_value, known_bad_value):
     """Given known good and bad values, decide if the current_value passed
     or failed.
 
     Args:
       current_value: The value of the metric being checked.
       known_bad_value: The reference value for a "failed" run.
       known_good_value: The reference value for a "passed" run.
 
     Returns:
       True if the current_value is closer to the known_good_value than the
       known_bad_value.
     """
-    dist_to_good_value = abs(current_value['mean'] - known_good_value['mean'])
-    dist_to_bad_value = abs(current_value['mean'] - known_bad_value['mean'])
+    if self.opts.bisect_mode == BISECT_MODE_STD_DEV:
+      dist_to_good_value = abs(current_value['std_dev'] -
+          known_good_value['std_dev'])
+      dist_to_bad_value = abs(current_value['std_dev'] -
+          known_bad_value['std_dev'])
+    else:
+      dist_to_good_value = abs(current_value['mean'] - known_good_value['mean'])
+      dist_to_bad_value = abs(current_value['mean'] - known_bad_value['mean'])
 
     return dist_to_good_value < dist_to_bad_value
 
   def _GetDepotDirectory(self, depot_name):
     if depot_name == 'chromium':
       return self.src_cwd
     elif depot_name == 'cros':
       return self.cros_cwd
     elif depot_name in DEPOT_NAMES:
       return self.depot_cwd[depot_name]
@@ skipping 535 matching lines @@
                                                    metric, skippable=True)
 
         # If the build is successful, check whether or not the metric
         # had regressed.
         if not run_results[1]:
           if len(run_results) > 2:
             next_revision_data['external'] = run_results[2]
             next_revision_data['perf_time'] = run_results[3]
             next_revision_data['build_time'] = run_results[4]
 
-          passed_regression = self.CheckIfRunPassed(run_results[0],
-                                                    known_good_value,
-                                                    known_bad_value)
+          passed_regression = self._CheckIfRunPassed(run_results[0],
+                                                     known_good_value,
+                                                     known_bad_value)
 
           next_revision_data['passed'] = passed_regression
           next_revision_data['value'] = run_results[0]
 
           if passed_regression:
             max_revision = next_revision_index
           else:
             min_revision = next_revision_index
         else:
           if run_results[1] == BUILD_RESULT_SKIPPED:
@@ skipping 34 matching lines @@
     # "Confidence in Bisection Results: 100%" to decide whether or not
     # to cc the author(s). If you change this, please update the perf
     # dashboard as well.
     print 'Confidence in Bisection Results: %d%%' % results_dict['confidence']
 
   def _PrintBanner(self, results_dict):
     print
     print " __o_\___          Aw Snap! We hit a speed bump!"
     print "=-O----O-'__.~.___________________________________"
     print
-    print 'Bisect reproduced a %.02f%% (+-%.02f%%) change in the %s metric.' % (
-        results_dict['regression_size'], results_dict['regression_std_err'],
-        '/'.join(self.opts.metric))
+    if self._IsBisectModeReturnCode():
+      print ('Bisect reproduced a change in return codes while running the '
+          'performance test.')
+    else:
+      print ('Bisect reproduced a %.02f%% (+-%.02f%%) change in the '
+          '%s metric.' % (results_dict['regression_size'],
+          results_dict['regression_std_err'], '/'.join(self.opts.metric)))
     self._PrintConfidence(results_dict)
 
   def _PrintFailedBanner(self, results_dict):
     print
-    print ('Bisect could not reproduce a change in the '
-        '%s/%s metric.' % (self.opts.metric[0], self.opts.metric[1]))
+    if self._IsBisectModeReturnCode():
+      print 'Bisect could not reproduce a change in the return code.'
+    else:
+      print ('Bisect could not reproduce a change in the '
+          '%s metric.' % '/'.join(self.opts.metric))
     print
-    self._PrintConfidence(results_dict)
 
   def _GetViewVCLinkFromDepotAndHash(self, cl, depot):
     info = self.source_control.QueryRevisionInfo(cl,
         self._GetDepotDirectory(depot))
     if depot and DEPOT_DEPS_NAME[depot].has_key('viewvc'):
       try:
         # Format is "git-svn-id: svn://....@123456 <other data>"
         svn_line = [i for i in info['body'].splitlines() if 'git-svn-id:' in i]
         svn_revision = svn_line[0].split('@')
         svn_revision = svn_revision[1].split(' ')[0]
@@ skipping 16 matching lines @@
       print 'Link    : %s' % commit_link
     else:
       print
       print 'Failed to parse svn revision from body:'
       print
       print info['body']
       print
     print 'Commit  : %s' % cl
     print 'Date    : %s' % info['date']
 
+  def _PrintTable(self, column_widths, row_data):

[tonyg, 2014/04/25 23:12:07]

Oops, I was thinking this would take all rows instead of one row. That way the
column widths would be tied together. But your way is fine too. If you keep this
factoring, I'd recommend just calling this PrintTableRow or something.

[shatch, 2014/04/25 23:15:10]

Done.

+    assert len(column_widths) == len(row_data)
+
+    text = ''
+    for i in xrange(len(column_widths)):
+      current_row_data = row_data[i].center(column_widths[i], ' ')
+      text += ('%%%ds' % column_widths[i]) % current_row_data
+    print text
+
+  def _PrintTestedCommitsHeader(self):
+    if self.opts.bisect_mode == BISECT_MODE_MEAN:
+      self._PrintTable(
+          [20, 70, 14, 12, 13],
+          ['Depot', 'Commit SHA', 'Mean', 'Std. Error', 'State'])
+    elif self.opts.bisect_mode == BISECT_MODE_STD_DEV:
+      self._PrintTable(
+          [20, 70, 14, 12, 13],
+          ['Depot', 'Commit SHA', 'Std. Error', 'Mean', 'State'])
+    elif self.opts.bisect_mode == BISECT_MODE_RETURN_CODE:
+      self._PrintTable(
+          [20, 70, 14, 13],
+          ['Depot', 'Commit SHA', 'Return Code', 'State'])
+    else:
+      assert False, "Invalid bisect_mode specified."
+      print '  %20s  %70s  %14s %13s' % ('Depot'.center(20, ' '),
+          'Commit SHA'.center(70, ' '), 'Return Code'.center(14, ' '),
+           'State'.center(13, ' '))
+
+  def _PrintTestedCommitsEntry(self, current_data, cl_link, state_str):
+    if self.opts.bisect_mode == BISECT_MODE_MEAN:
+      std_error = '+-%.02f' % current_data['value']['std_err']
+      mean = '%.02f' % current_data['value']['mean']
+      self._PrintTable(
+          [20, 70, 12, 14, 13],
+          [current_data['depot'], cl_link, mean, std_error, state_str])
+    elif self.opts.bisect_mode == BISECT_MODE_STD_DEV:
+      std_error = '+-%.02f' % current_data['value']['std_err']
+      mean = '%.02f' % current_data['value']['mean']
+      self._PrintTable(
+          [20, 70, 12, 14, 13],
+          [current_data['depot'], cl_link, std_error, mean, state_str])
+    elif self.opts.bisect_mode == BISECT_MODE_RETURN_CODE:
+      mean = '%d' % current_data['value']['mean']
+      self._PrintTable(
+          [20, 70, 14, 13],
+          [current_data['depot'], cl_link, mean, state_str])
+
   def _PrintTestedCommitsTable(self, revision_data_sorted,
       first_working_revision, last_broken_revision, confidence,
       final_step=True):
     print
     if final_step:
       print 'Tested commits:'
     else:
       print 'Partial results:'
-    print '  %20s  %70s  %12s %14s %13s' % ('Depot'.center(20, ' '),
-        'Commit SHA'.center(70, ' '), 'Mean'.center(12, ' '),
-        'Std. Error'.center(14, ' '), 'State'.center(13, ' '))
+    self._PrintTestedCommitsHeader()
     state = 0
     for current_id, current_data in revision_data_sorted:
       if current_data['value']:
         if (current_id == last_broken_revision or
             current_id == first_working_revision):
           # If confidence is too low, don't add this empty line since it's
           # used to put focus on a suspected CL.
           if confidence and final_step:
             print
           state += 1
           if state == 2 and not final_step:
             # Just want a separation between "bad" and "good" cl's.
             print
 
         state_str = 'Bad'
         if state == 1 and final_step:
           state_str = 'Suspected CL'
         elif state == 2:
           state_str = 'Good'
 
         # If confidence is too low, don't bother outputting good/bad.
         if not confidence:
           state_str = ''
         state_str = state_str.center(13, ' ')
 
-        std_error = ('+-%.02f' %
-            current_data['value']['std_err']).center(14, ' ')
-        mean = ('%.02f' % current_data['value']['mean']).center(12, ' ')
         cl_link = self._GetViewVCLinkFromDepotAndHash(current_id,
             current_data['depot'])
         if not cl_link:
           cl_link = current_id
-        print '  %20s  %70s  %12s %14s %13s' % (
-            current_data['depot'].center(20, ' '), cl_link.center(70, ' '),
-            mean, std_error, state_str)
+        self._PrintTestedCommitsEntry(current_data, cl_link, state_str)
 
   def _PrintReproSteps(self):
     print
     print 'To reproduce locally:'
     print '$ ' + self.opts.command
     if bisect_utils.IsTelemetryCommand(self.opts.command):
       print
       print 'Also consider passing --profiler=list to see available profilers.'
 
   def _PrintOtherRegressions(self, other_regressions, revision_data):
@@ skipping 354 matching lines @@
     self.no_custom_deps = False
     self.working_directory = None
     self.extra_src = None
     self.debug_ignore_build = None
     self.debug_ignore_sync = None
     self.debug_ignore_perf_test = None
     self.gs_bucket = None
     self.target_arch = 'ia32'
     self.builder_host = None
     self.builder_port = None
+    self.bisect_mode = BISECT_MODE_MEAN
 
   def _CreateCommandLineParser(self):
     """Creates a parser with bisect options.
 
     Returns:
       An instance of optparse.OptionParser.
     """
     usage = ('%prog [options] [-- chromium-options]\n'
              'Perform binary search on revision history to find a minimal '
              'range of revisions where a peformance metric regressed.\n')
@@ skipping 34 matching lines @@
                      'doesn\'t exceed --max_time_minutes. Values will be '
                      'clamped to range [1, 60].'
                      'Default value is 20.')
     group.add_option('-t', '--truncate_percent',
                      type='int',
                      default=25,
                      help='The highest/lowest % are discarded to form a '
                      'truncated mean. Values will be clamped to range [0, '
                      '25]. Default value is 25 (highest/lowest 25% will be '
                      'discarded).')
+    group.add_option('--bisect_mode',
+                     type='choice',
+                     choices=[BISECT_MODE_MEAN, BISECT_MODE_STD_DEV,
+                        BISECT_MODE_RETURN_CODE],
+                     default=BISECT_MODE_MEAN,
+                     help='The bisect mode. Choices are to bisect on the '
+                     'difference in mean, std_dev, or return_code.')
     parser.add_option_group(group)
 
     group = optparse.OptionGroup(parser, 'Build options')
     group.add_option('-w', '--working_directory',
                      type='str',
                      help='Path to the working directory where the script '
                      'will do an initial checkout of the chromium depot. The '
                      'files will be placed in a subdirectory "bisect" under '
                      'working_directory and that will be used to perform the '
                      'bisection. This parameter is optional, if it is not '
@@ skipping 79 matching lines @@
     try:
       if not opts.command:
         raise RuntimeError('missing required parameter: --command')
 
       if not opts.good_revision:
         raise RuntimeError('missing required parameter: --good_revision')
 
       if not opts.bad_revision:
         raise RuntimeError('missing required parameter: --bad_revision')
 
-      if not opts.metric:
+      if not opts.metric and opts.bisect_mode != BISECT_MODE_RETURN_CODE:
         raise RuntimeError('missing required parameter: --metric')
 
       if opts.gs_bucket:
         if not cloud_storage.List(opts.gs_bucket):
           raise RuntimeError('Invalid Google Storage: gs://%s' % opts.gs_bucket)
         if not opts.builder_host:
           raise RuntimeError('Must specify try server hostname, when '
                              'gs_bucket is used: --builder_host')
         if not opts.builder_port:
           raise RuntimeError('Must specify try server port number, when '
                              'gs_bucket is used: --builder_port')
       if opts.target_platform == 'cros':
         # Run sudo up front to make sure credentials are cached for later.
         print 'Sudo is required to build cros:'
         print
         RunProcess(['sudo', 'true'])
 
         if not opts.cros_board:
           raise RuntimeError('missing required parameter: --cros_board')
 
         if not opts.cros_remote_ip:
           raise RuntimeError('missing required parameter: --cros_remote_ip')
 
         if not opts.working_directory:
           raise RuntimeError('missing required parameter: --working_directory')
 
       metric_values = opts.metric.split('/')
-      if len(metric_values) != 2:
+      if (len(metric_values) != 2 and
+          opts.bisect_mode != BISECT_MODE_RETURN_CODE):
         raise RuntimeError("Invalid metric specified: [%s]" % opts.metric)
 
       opts.metric = metric_values
       opts.repeat_test_count = min(max(opts.repeat_test_count, 1), 100)
       opts.max_time_minutes = min(max(opts.max_time_minutes, 1), 60)
       opts.truncate_percent = min(max(opts.truncate_percent, 0), 25)
       opts.truncate_percent = opts.truncate_percent / 100.0
 
       for k, v in opts.__dict__.iteritems():
         assert hasattr(self, k), "Invalid %s attribute in BisectOptions." % k
@@ skipping 91 matching lines @@
       # The perf dashboard scrapes the "results" step in order to comment on
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 if __name__ == '__main__':
   sys.exit(main())