Obtain confidence score based off last known good and first known bad revision results.

tools/auto_bisect/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 59 matching lines @@
 # Maximum time in seconds to wait after posting build request to the try server.
 # TODO: Change these values based on the actual time taken by buildbots on
 # the try server.
 MAX_MAC_BUILD_TIME = 14400
 MAX_WIN_BUILD_TIME = 14400
 MAX_LINUX_BUILD_TIME = 14400
 
 # The confidence percentage we require to consider the initial range a
 # regression based on the test results of the inital good and bad revisions.
 REGRESSION_CONFIDENCE = 80
+# How many times to repeat the test on the last known good and first known bad
+# revisions in order to assess a more accurate confidence score in the
+# regression culprit.
+BORDER_REVISIONS_EXTRA_RUNS = 2
 
 # Patch template to add a new file, DEPS.sha under src folder.
 # This file contains SHA1 value of the DEPS changes made while bisecting
 # dependency repositories. This patch send along with DEPS patch to try server.
 # When a build requested is posted with a patch, bisect builders on try server,
 # once build is produced, it reads SHA value from this file and appends it
 # to build archive filename.
 DEPS_SHA_PATCH = """diff --git DEPS.sha DEPS.sha
 new file mode 100644
 --- /dev/null
@@ skipping 1136 matching lines @@
           return command_to_run.replace(cmd_browser,
                                         'android-chromium-testshell')
         elif (commit_position >= 276628 and
               cmd_browser == 'android-chromium-testshell'):
           return command_to_run.replace(cmd_browser,
                                         'android-chrome-shell')
     return command_to_run
 
   def RunPerformanceTestAndParseResults(
       self, command_to_run, metric, reset_on_first_run=False,
-      upload_on_last_run=False, results_label=None):
+      upload_on_last_run=False, results_label=None, extra_test_runs=0):
     """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.
           May be None for perf try jobs.
       reset_on_first_run: If True, pass the flag --reset-results on first run.
       upload_on_last_run: If True, pass the flag --upload-results on last run.
       results_label: A value for the option flag --results-label.
           The arguments reset_on_first_run, upload_on_last_run and results_label
           are all ignored if the test is not a Telemetry test.
+      extra_test_runs: Factor by which to increase the number of test repeats
+          and the timeout period specified in self.opts.

[qyearsley, 2015/01/16 18:37:03]

Possible suggestion:
Rename this to "repeat_count_multiplier" (or "test_run_multiplier"), give it a
default value of 1 in the args of RunTest, and change below to:
for i in xrange(self.opts.repeat_test_count * repeat_count_multiplier):
  ...
And change the time limit in the same way.

Would this be clearer or no?

[RobertoCN, 2015/01/30 21:23:32]

Done.

 
     Returns:
       (values dict, 0) if --debug_ignore_perf_test was passed.
       (values dict, 0, test output) if the test was run successfully.
       (error message, -1) if the test couldn't be run.
       (error message, -1, test output) if the test ran but there was an error.
     """
     success_code, failure_code = 0, -1
 
     if self.opts.debug_ignore_perf_test:
@@ skipping 21 matching lines @@
     if not _GenerateProfileIfNecessary(args):
       err_text = 'Failed to generate profile for performance test.'
       return (err_text, failure_code)
 
     is_telemetry = bisect_utils.IsTelemetryCommand(command_to_run)
 
     start_time = time.time()
 
     metric_values = []
     output_of_all_runs = ''
-    for i in xrange(self.opts.repeat_test_count):
+    repeat_count = self.opts.repeat_test_count * (1 + extra_test_runs)
+    for i in xrange(repeat_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')
         if 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:
@@ skipping 20 matching lines @@
         metric_values.append(math_utils.Mean(
             _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:
+      time_limit = self.opts.max_time_minutes *  (1 + extra_test_runs)
+      if elapsed_minutes >= time_limit:
         break
 
     if metric and 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)
 
     # If we're bisecting on return codes, we're really just looking for zero vs
     # non-zero.
@@ skipping 84 matching lines @@
       output = bisect_utils.CheckRunGit(cmd)
 
       files = output.splitlines()
 
       if len(files) == 1 and files[0] == 'DEPS':
         return True
 
     return False
 
   def RunTest(self, revision, depot, command, metric, skippable=False,
-      skip_sync=False, create_patch=False, force_build=False):
+              skip_sync=False, create_patch=False, force_build=False,
+              extra_test_runs=0):
     """Performs a full sync/build/run of the specified revision.
 
     Args:
       revision: The revision to sync to.
       depot: The depot that's being used at the moment (src, webkit, etc.)
       command: The command to execute the performance test.
       metric: The performance metric being tested.
       skip_sync: Skip the sync step.
       create_patch: Create a patch with any locally modified files.
       force_build: Force a local build.
+      extra_test_runs: Factor by which to increase the given number of runs and
+          the set timeout period.
 
     Returns:
       On success, a tuple containing the results of the performance test.
       Otherwise, a tuple with the error message.
     """
     logging.info('Running RunTest with rev "%s", command "%s"',
                  revision, command)
     # Decide which sync program to use.
     sync_client = None
     if depot == 'chromium' or depot == 'android-chrome':
@@ skipping 21 matching lines @@
         depot, revision=revision_to_build, create_patch=create_patch)
     if not build_success:
       return ('Failed to build revision: [%s]' % str(revision),
               BUILD_RESULT_FAIL)
     after_build_time = time.time()
 
     # Possibly alter the command.
     command = self.GetCompatibleCommand(command, revision, depot)
 
     # Run the command and get the results.
-    results = self.RunPerformanceTestAndParseResults(command, metric)
+    results = self.RunPerformanceTestAndParseResults(
+        command, metric, extra_test_runs=extra_test_runs)
 
     # Restore build output directory once the tests are done, to avoid
     # any discrepancies.
     if self.IsDownloadable(depot) and revision:
       self.BackupOrRestoreOutputDirectory(restore=True)
 
     # A value other than 0 indicates that the test couldn't be run, and results
     # should also include an error message.
     if results[1] != 0:
       return results
@@ skipping 893 matching lines @@
 
           # If the build is broken, remove it and redo search.
           revision_states.pop(next_revision_index)
 
           max_revision -= 1
 
         if self.opts.output_buildbot_annotations:
           self.printer.PrintPartialResults(bisect_state)
           bisect_utils.OutputAnnotationStepClosed()
 
+
+      self._ConfidenceExtraTestRuns(min_revision_state, max_revision_state,
+                                    command_to_run, metric)
       results = BisectResults(bisect_state, self.depot_registry, self.opts,
                               self.warnings)
 
       self._GatherResultsFromRevertedCulpritCL(
           results, target_depot, command_to_run, metric)
 
       return results
     else:
       # Weren't able to sync and retrieve the revision range.
       error = ('An error occurred attempting to retrieve revision range: '
                '[%s..%s]' % (good_revision, bad_revision))
       return BisectResults(error=error)
 
+  def _ConfidenceExtraTestRuns(self, good_state, bad_state, command_to_run,
+                               metric):
+    if(bool(good_state.passed) != bool(bad_state.passed)

[qyearsley, 2015/01/16 18:37:03]

Add space after "if"

[RobertoCN, 2015/01/30 21:23:31]

Done.

+       and good_state.passed not in ('Skipped', 'Build Failed')
+       and bad_state.passed not in ('Skipped', 'Build Failed')):
+      for state in (good_state, bad_state):
+        run_results = self.RunTest(
+            state.revision,
+            state.depot,
+            command_to_run,
+            metric,
+            extra_test_runs=BORDER_REVISIONS_EXTRA_RUNS
+        )

[qyearsley, 2015/01/16 18:37:03]

Since this is a function call, the convention is to put the closing parenthesis
on the same line as the last arg.

[RobertoCN, 2015/01/30 21:23:32]

Done.

+        # Is extend the right thing to do here?
+        state.value['values'].extend(run_results[0]['values'])
+      # Get new confidence score

[RobertoCN, 2015/01/14 18:50:47]

This comment will be removed. 

[RobertoCN, 2015/01/30 21:23:32]

Done.

+
 
 def _IsPlatformSupported():
   """Checks that this platform and build system are supported.
 
   Args:
     opts: The options parsed from the command line.
 
   Returns:
     True if the platform and build system are supported.
   """
@@ skipping 355 matching lines @@
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Runtime Error: %s' % e
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())