Try to expand the revision range to include a change to .DEPS.git if the script detects a change to…

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 233 matching lines @@
     command: A list containing the args to git.
 
   Returns:
     A tuple of the output and return code.
   """
   command = ['git'] + command
 
   return RunProcess(command)
 
 
+def CheckRunGit(command):
+  """Run a git subcommand, returning its output and return code. Asserts if
+  the return code of the call is non-zero.
+
+  Args:
+    command: A list containing the args to git.
+
+  Returns:
+    A tuple of the output and return code.
+  """
+  (output, return_code) = RunGit(command)
+
+  assert not return_code, 'An error occurred while running'\
+                          ' "git %s"' % ' '.join(command)
+  return (output, return_code)

[tonyg, 2013/05/28 23:45:06]

Since we know return_code is 0 at this point, I'd just return output instead of
returning a tuple. It will make the callsites cleaner too.

[shatch, 2013/05/29 16:25:28]

Done.

+
+
 def BuildWithMake(threads, targets, print_output):
   cmd = ['make', 'BUILDTYPE=Release', '-j%d' % threads] + targets
 
   (output, return_code) = RunProcess(cmd, print_output)
 
   return not return_code
 
 
 def BuildWithNinja(threads, targets, print_output):
   cmd = ['ninja', '-C', os.path.join('out', 'Release'),
@@ skipping 55 matching lines @@
     Args:
       revision_range_end: The SHA1 for the end of the range.
       revision_range_start: The SHA1 for the beginning of the range.
 
     Returns:
       A list of the revisions between |revision_range_start| and
       |revision_range_end| (inclusive).
     """
     revision_range = '%s..%s' % (revision_range_start, revision_range_end)
     cmd = ['log', '--format=%H', '-10000', '--first-parent', revision_range]
-    (log_output, return_code) = RunGit(cmd)
-
-    assert not return_code, 'An error occurred while running'\
-                            ' "git %s"' % ' '.join(cmd)
+    (log_output, return_code) = CheckRunGit(cmd)
 
     revision_hash_list = log_output.split()
     revision_hash_list.append(revision_range_start)
 
     return revision_hash_list
 
   def SyncToRevision(self, revision, use_gclient=True):
     """Syncs to the specified revision.
 
     Args:
       revision: The revision to sync to.
       use_gclient: Specifies whether or not we should sync using gclient or
         just use source control directly.
 
     Returns:
       True if successful.
     """
 
     if use_gclient:
       results = self.SyncToRevisionWithGClient(revision)
     else:
       results = RunGit(['checkout', revision])[1]

[tonyg, 2013/05/28 23:45:06]

CheckRunGit?

[shatch, 2013/05/29 16:25:28]

Not sure under which conditions checkout can fail. At the moment, the script
will continue and mark that build as bad, changing it to CheckRunGit would
assert and stop the run completely.

 
     return not results
 
   def ResolveToRevision(self, revision_to_check, depot, search):
     """If an SVN revision is supplied, try to resolve it to a git SHA1.
 
     Args:
       revision_to_check: The user supplied revision string that may need to be
         resolved to a git SHA1.
       depot: The depot the revision_to_check is from.
@@ skipping 17 matching lines @@
 
     if search > 0:
       search_range = xrange(svn_revision, svn_revision + search, 1)
     else:
       search_range = xrange(svn_revision, svn_revision + search, -1)
 
     for i in search_range:
       svn_pattern = 'git-svn-id: %s@%d' % (depot_svn, i)
       cmd = ['log', '--format=%H', '-1', '--grep', svn_pattern, 'origin/master']
 
-      (log_output, return_code) = RunGit(cmd)
-
-      assert not return_code, 'An error occurred while running'\
-                              ' "git %s"' % ' '.join(cmd)
+      (log_output, return_code) = CheckRunGit(cmd)
 
       if not return_code:
         log_output = log_output.strip()
 
         if log_output:
           git_revision = log_output
 
           break
 
     return git_revision
 
   def IsInProperBranch(self):
     """Confirms they're in the master branch for performing the bisection.
     This is needed or gclient will fail to sync properly.
 
     Returns:
       True if the current branch on src is 'master'
     """
     cmd = ['rev-parse', '--abbrev-ref', 'HEAD']
-    (log_output, return_code) = RunGit(cmd)
-
-    assert not return_code, 'An error occurred while running'\
-                            ' "git %s"' % ' '.join(cmd)
+    (log_output, return_code) = CheckRunGit(cmd)
 
     log_output = log_output.strip()
 
     return log_output == "master"
 
   def SVNFindRev(self, revision):
     """Maps directly to the 'git svn find-rev' command.
 
     Args:
       revision: The git SHA1 to use.
 
     Returns:
       An integer changelist #, otherwise None.
     """
 
     cmd = ['svn', 'find-rev', revision]
 
-    (output, return_code) = RunGit(cmd)
-
-    assert not return_code, 'An error occurred while running'\
-                            ' "git %s"' % ' '.join(cmd)
+    (output, return_code) = CheckRunGit(cmd)
 
     svn_revision = output.strip()
 
     if IsStringInt(svn_revision):
       return int(svn_revision)
 
     return None
 
   def QueryRevisionInfo(self, revision):
     """Gathers information on a particular revision, such as author's name,
@@ skipping 10 matching lines @@
         'subject': %s,
       }
     """
     commit_info = {}
 
     formats = ['%cN', '%cE', '%s', '%cD']
     targets = ['author', 'email', 'subject', 'date']
 
     for i in xrange(len(formats)):
       cmd = ['log', '--format=%s' % formats[i], '-1', revision]
-      (output, return_code) = RunGit(cmd)
+      (output, return_code) = CheckRunGit(cmd)
       commit_info[targets[i]] = output.rstrip()
 
-      assert not return_code, 'An error occurred while running'\
-                              ' "git %s"' % ' '.join(cmd)
-
     return commit_info
 
   def CheckoutFileAtRevision(self, file_name, revision):
     """Performs a checkout on a file at the given revision.
 
     Returns:
       True if successful.
     """
     return not RunGit(['checkout', revision, file_name])[1]
 
   def RevertFileToHead(self, file_name):
     """Unstages a file and returns it to HEAD.
 
     Returns:
       True if successful.
     """
     # Reset doesn't seem to return 0 on success.
     RunGit(['reset', 'HEAD', bisect_utils.FILE_DEPS_GIT])
 
     return not RunGit(['checkout', bisect_utils.FILE_DEPS_GIT])[1]
 
+  def QueryFileRevisionHistory(self, filename, revision_start, revision_end):
+    """Returns a list of commits that modified this file.
+
+    Args:
+        filename: Name of file.
+        revision_start: Start of revision range.
+        revision_end: End of revision range.
+
+    Returns:
+        Returns a list of commits that touched this file.
+    """
+    cmd = ['log', '--format=%H', '%s^1..%s' % (revision_start, revision_end),
+           filename]
+    (output, return_code) = CheckRunGit(cmd)
+
+    return [o for o in output.split('\n') if o]
+
 class BisectPerformanceMetrics(object):
   """BisectPerformanceMetrics performs a bisection against a list of range
   of revisions to narrow down where performance regressions may have
   occurred."""
 
   def __init__(self, source_control, opts):
     super(BisectPerformanceMetrics, self).__init__()
 
     self.opts = opts
     self.source_control = source_control
     self.src_cwd = os.getcwd()
     self.depot_cwd = {}
     self.cleanup_commands = []
+    self.warnings = []
 
     # This always starts true since the script grabs latest first.
     self.was_blink = True
 
     for d in DEPOT_NAMES:
       # The working directory of each depot is just the path to the depot, but
       # since we're already in 'src', we can skip that part.
 
       self.depot_cwd[d] = self.src_cwd + DEPOT_DEPS_NAME[d]['src'][3:]
 
@@ skipping 517 matching lines @@
 
     print
     print 'Revisions to bisect on [%s]:' % depot
     for revision_id in revision_list:
       print '  -> %s' % (revision_id, )
     print
 
     if self.opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
 
+  def NudgeRevisionsIfDEPSChange(self, bad_revision, good_revision):
+    """Checks to see if changes to DEPS file occurred, and that the revision
+    range also includes the change to .DEPS.git. If it doesn't, attempts to
+    expand the revision range to include it.
+
+    Args:
+        bad_rev: First known bad revision.
+        good_revision: Last known good revision.
+
+    Returns:
+        A tuple with the new bad and good revisions.
+    """
+    if self.source_control.IsGit():
+      changes_to_deps = self.source_control.QueryFileRevisionHistory(
+          'DEPS', good_revision, bad_revision)
+
+      if changes_to_deps:
+        # DEPS file was changed, search from the oldest change to DEPS file to
+        # bad_revision to see if there are matching .DEPS.git changes.
+        oldest_deps_change = changes_to_deps[-1]
+        changes_to_gitdeps = self.source_control.QueryFileRevisionHistory(
+            bisect_utils.FILE_DEPS_GIT, oldest_deps_change, bad_revision)
+
+        if len(changes_to_deps) != len(changes_to_gitdeps):
+          # Grab the timestamp of the last DEPS change
+          cmd = ['log', '--format=%ct', '-1', changes_to_deps[0]]
+          (output, return_code) = CheckRunGit(cmd)
+
+          commit_time = int(output)
+
+          # Try looking for a commit that touches the .DEPS.git file in the
+          # next 15 minutes after the DEPS file change.
+          cmd = ['log', '--format=%H', '-1',
+              '--before=%d' % (commit_time + 900), '--after=%d' % commit_time,
+              'origin/master', bisect_utils.FILE_DEPS_GIT]
+          (output, return_code) = CheckRunGit(cmd)
+
+          output = output.strip()
+          if output:
+            self.warnings.append('Detected change to DEPS and modified '
+                'revision range to include change to .DEPS.git')
+            return (output, good_revision)
+          else:
+            self.warnings.append('Detected change to DEPS but couldn\'t find '
+                'matching change to .DEPS.git')
+    return (bad_revision, good_revision)
+
   def Run(self, command_to_run, bad_revision_in, good_revision_in, metric):
     """Given known good and bad revisions, run a binary search on all
     intermediate revisions to determine the CL where the performance regression
     occurred.
 
     Args:
         command_to_run: Specify the command to execute the performance test.
         good_revision: Number/tag of the known good revision.
         bad_revision: Number/tag of the known bad revision.
         metric: The performance metric to monitor.
@@ skipping 41 matching lines @@
                                                           'src', -100)
 
     if bad_revision is None:
       results['error'] = 'Could\'t resolve [%s] to SHA1.' % (bad_revision_in,)
       return results
 
     if good_revision is None:
       results['error'] = 'Could\'t resolve [%s] to SHA1.' % (good_revision_in,)
       return results
 
+    (bad_revision, good_revision) = self.NudgeRevisionsIfDEPSChange(
+        bad_revision, good_revision)
+
     if self.opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepStart('Gathering Revisions')
 
     print 'Gathering revision range for bisection.'
 
     # Retrieve a list of revisions to do bisection on.
     src_revision_list = self.GetRevisionList(bad_revision, good_revision)
 
     if self.opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
@@ skipping 255 matching lines @@
       good_std_dev = revision_data[first_working_revision]['value']['std_dev']
       good_mean = revision_data[first_working_revision]['value']['mean']
       bad_mean = revision_data[last_broken_revision]['value']['mean']
 
       # A standard deviation of 0 could indicate either insufficient runs
       # or a test that consistently returns the same value.
       if good_std_dev > 0:
         deviations = math.fabs(bad_mean - good_mean) / good_std_dev
 
         if deviations < 1.5:
-          print 'Warning: Regression was less than 1.5 standard deviations '\
-                'from "good" value. Results may not be accurate.'
-          print
+          self.warnings.append('Regression was less than 1.5 standard '
+            'deviations from "good" value. Results may not be accurate.')
       elif self.opts.repeat_test_count == 1:
-        print 'Warning: Tests were only set to run once. This may be '\
-              'insufficient to get meaningful results.'
-        print
+        self.warnings.append('Tests were only set to run once. This '
+            'may be insufficient to get meaningful results.')
 
       # Check for any other possible regression ranges
       prev_revision_data = revision_data_sorted[0][1]
       prev_revision_id = revision_data_sorted[0][0]
       possible_regressions = []
       for current_id, current_data in revision_data_sorted:
         if current_data['value']:
           prev_mean = prev_revision_data['value']['mean']
           cur_mean = current_data['value']['mean']
 
@@ skipping 40 matching lines @@
           if percent_change is None:
             percent_change = 0
 
           print '  %8s  %s  [%.2f%%, %s x std.dev]' % (
               previous_data['depot'], previous_id, 100 * percent_change,
               deviations)
           print '  %8s  %s' % (
               current_data['depot'], current_id)
           print
 
+      if self.warnings:
+        print
+        print 'The following warnings were generated:'
+        print
+        for w in self.warnings:
+          print '  - %s' % w
+        print
+
     if self.opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
 
 
 def DetermineAndCreateSourceControl():
   """Attempts to determine the underlying source control workflow and returns
   a SourceControl object.
 
   Returns:
     An instance of a SourceControl object, or None if the current workflow
     is unsupported.
   """
 
   (output, return_code) = RunGit(['rev-parse', '--is-inside-work-tree'])

[tonyg, 2013/05/28 23:45:06]

CheckRunGit?

[shatch, 2013/05/29 16:25:28]

Prefer not to here, since a more descriptive error message is displayed after if
this function fails.

 
   if output.strip() == 'true':
     return GitSourceControl()
 
   return None
 
 
 def SetNinjaBuildSystemDefault():
   """Makes ninja the default build system to be used by
   the bisection script."""
@@ skipping 255 matching lines @@
 
   if not(bisect_results['error']):
     return 0
   else:
     print 'Error: ' + bisect_results['error']
     print
     return 1
 
 if __name__ == '__main__':
   sys.exit(main())