If V8 revisions aren't mappable to bleeding_edge, try to search for one that is

This came out of a discussion with danno, machenbach, and jkummerow on how to improve the current V8 bisects. At the moment, the bisect checks if both end points of a v8 bisect are directly mappable to bleeding_edge revisions and bails if not. Instead, it will not try to search forward/backwards a few revisions to see if it can find a CL that is mappable to bleeding_edge and use that new range instead. In that situation, the bisect will generate a warning that it had to search for a new endpoint and the results might not be valid.

BUG=
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232859

[shatch, 2013-11-04 16:49:25]

On 2013/10/29 22:56:03, shatch wrote:

ping ptal

[tonyg, 2013-11-04 17:26:38]

lgtm

Really sorry I dropped this for so long. Thanks for doing this.

[commit-bot: I haz the power, 2013-11-04 18:29:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/50043003/1

[commit-bot: I haz the power, 2013-11-04 18:29:15]

Failed to apply patch for tools/bisect-perf-regression.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/bisect-perf-regression.py
  Hunk #15 FAILED at 2301.
  1 out of 15 hunks FAILED -- saving rejects to file
tools/bisect-perf-regression.py.rej

Patch:       tools/bisect-perf-regression.py
Index: tools/bisect-perf-regression.py
diff --git a/tools/bisect-perf-regression.py b/tools/bisect-perf-regression.py
index
a5b87f980837c497681a70837172128d72147eb3..7ec156fa270efa487e04ec3e7f151b7413a80aa0
100755
--- a/tools/bisect-perf-regression.py
+++ b/tools/bisect-perf-regression.py
@@ -278,7 +278,7 @@ def RunProcess(command):
   return subprocess.call(command, shell=shell)
 
 
-def RunProcessAndRetrieveOutput(command):
+def RunProcessAndRetrieveOutput(command, cwd=None):
   """Run an arbitrary command, returning its output and return code. Since
   output is collected via communicate(), there will be no output until the
   call terminates. If you need output while the program runs (ie. so
@@ -286,24 +286,20 @@ def RunProcessAndRetrieveOutput(command):
 
   Args:
     command: A list containing the command and args to execute.
-    print_output: Optional parameter to write output to stdout as it's
-        being collected.
 
   Returns:
     A tuple of the output and return code.
   """
   # On Windows, use shell=True to get PATH interpretation.
   shell = IsWindows()
-  proc = subprocess.Popen(command,
-                          shell=shell,
-                          stdout=subprocess.PIPE)
+  proc = subprocess.Popen(command, shell=shell, stdout=subprocess.PIPE,
cwd=cwd)
 
   (output, _) = proc.communicate()
 
   return (output, proc.returncode)
 
 
-def RunGit(command):
+def RunGit(command, cwd=None):
   """Run a git subcommand, returning its output and return code.
 
   Args:
@@ -314,10 +310,10 @@ def RunGit(command):
   """
   command = ['git'] + command
 
-  return RunProcessAndRetrieveOutput(command)
+  return RunProcessAndRetrieveOutput(command, cwd=cwd)
 
 
-def CheckRunGit(command):
+def CheckRunGit(command, cwd=None):
   """Run a git subcommand, returning its output and return code. Asserts if
   the return code of the call is non-zero.
 
@@ -327,7 +323,7 @@ def CheckRunGit(command):
   Returns:
     A tuple of the output and return code.
   """
-  (output, return_code) = RunGit(command)
+  (output, return_code) = RunGit(command, cwd=cwd)
 
   assert not return_code, 'An error occurred while running'\
                           ' "git %s"' % ' '.join(command)
@@ -717,7 +713,7 @@ class GitSourceControl(SourceControl):
 
     return not results
 
-  def ResolveToRevision(self, revision_to_check, depot, search):
+  def ResolveToRevision(self, revision_to_check, depot, search, cwd=None):
     """If an SVN revision is supplied, try to resolve it to a git SHA1.
 
     Args:
@@ -753,7 +749,7 @@ class GitSourceControl(SourceControl):
         cmd = ['log', '--format=%H', '-1', '--grep', svn_pattern,
                'origin/master']
 
-        (log_output, return_code) = RunGit(cmd)
+        (log_output, return_code) = RunGit(cmd, cwd=cwd)
 
         assert not return_code, 'An error occurred while running'\
                                 ' "git %s"' % ' '.join(cmd)
@@ -779,7 +775,7 @@ class GitSourceControl(SourceControl):
 
         git_revision = None
 
-        log_output = CheckRunGit(cmd)
+        log_output = CheckRunGit(cmd, cwd=cwd)
         if log_output:
           git_revision = log_output
           git_revision = int(log_output.strip())
@@ -820,7 +816,7 @@ class GitSourceControl(SourceControl):
 
     return None
 
-  def QueryRevisionInfo(self, revision):
+  def QueryRevisionInfo(self, revision, cwd=None):
     """Gathers information on a particular revision, such as author's name,
     email, subject, and date.
 
@@ -843,7 +839,7 @@ class GitSourceControl(SourceControl):
 
     for i in xrange(len(formats)):
       cmd = ['log', '--format=%s' % formats[i], '-1', revision]
-      output = CheckRunGit(cmd)
+      output = CheckRunGit(cmd, cwd=cwd)
       commit_info[targets[i]] = output.rstrip()
 
     return commit_info
@@ -956,6 +952,71 @@ class BisectPerformanceMetrics(object):
 
     return revision_work_list
 
+  def _GetV8BleedingEdgeFromV8TrunkIfMappable(self, revision):
+    svn_revision = self.source_control.SVNFindRev(revision)
+
+    if IsStringInt(svn_revision):
+      # V8 is tricky to bisect, in that there are only a few instances when
+      # we can dive into bleeding_edge and get back a meaningful result.
+      # Try to detect a V8 "business as usual" case, which is when:
+      #  1. trunk revision N has description "Version X.Y.Z"
+      #  2. bleeding_edge revision (N-1) has description "Prepare push to
+      #     trunk. Now working on X.Y.(Z+1)."
+      v8_dir = self._GetDepotDirectory('v8')
+      v8_bleeding_edge_dir = self._GetDepotDirectory('v8_bleeding_edge')
+
+      revision_info = self.source_control.QueryRevisionInfo(revision,
+          cwd=v8_dir)
+
+      version_re = re.compile("Version (?P<values>[0-9,.]+)")
+
+      regex_results = version_re.search(revision_info['subject'])
+
+      if regex_results:
+        version = regex_results.group('values')
+
+        git_revision = self.source_control.ResolveToRevision(
+            int(svn_revision) - 1, 'v8_bleeding_edge', -1,
+            cwd=v8_bleeding_edge_dir)
+
+        if git_revision:
+          revision_info = self.source_control.QueryRevisionInfo(git_revision,
+              cwd=v8_bleeding_edge_dir)
+
+          if 'Prepare push to trunk' in revision_info['subject']:
+            return git_revision
+    return None
+
+  def _GetNearestV8BleedingEdgeFromTrunk(self, revision, search_forward=True):
+    cwd = self._GetDepotDirectory('v8')
+    cmd = ['log', '--format=%ct', '-1', revision]
+    output = CheckRunGit(cmd, cwd=cwd)
+    commit_time = int(output)
+    commits = []
+
+    if search_forward:
+      cmd = ['log', '--format=%H', '-10', '--after=%d' % commit_time,
+          'origin/master']
+      output = CheckRunGit(cmd, cwd=cwd)
+      output = output.split()
+      commits = output
+      commits = reversed(commits)
+    else:
+      cmd = ['log', '--format=%H', '-10', '--before=%d' % commit_time,
+          'origin/master']
+      output = CheckRunGit(cmd, cwd=cwd)
+      output = output.split()
+      commits = output
+
+    bleeding_edge_revision = None
+
+    for c in commits:
+      bleeding_edge_revision = self._GetV8BleedingEdgeFromV8TrunkIfMappable(c)
+      if bleeding_edge_revision:
+        break
+
+    return bleeding_edge_revision
+
   def Get3rdPartyRevisionsFromCurrentRevision(self, depot, revision):
     """Parses the DEPS file to determine WebKit/v8/etc... versions.
 
@@ -1041,39 +1102,11 @@ class BisectPerformanceMetrics(object):
 
           results['chromium'] = output.strip()
     elif depot == 'v8':
+      # We can't try to map the trunk revision to bleeding edge yet, because
+      # we don't know which direction to try to search in. Have to wait until
+      # the bisect has narrowed the results down to 2 v8 rolls.
       results['v8_bleeding_edge'] = None
 
-      svn_revision = self.source_control.SVNFindRev(revision)
-
-      if IsStringInt(svn_revision):
-        # V8 is tricky to bisect, in that there are only a few instances when
-        # we can dive into bleeding_edge and get back a meaningful result.
-        # Try to detect a V8 "business as usual" case, which is when:
-        #  1. trunk revision N has description "Version X.Y.Z"
-        #  2. bleeding_edge revision (N-1) has description "Prepare push to
-        #     trunk. Now working on X.Y.(Z+1)."
-        self.ChangeToDepotWorkingDirectory(depot)
-
-        revision_info = self.source_control.QueryRevisionInfo(revision)
-
-        version_re = re.compile("Version (?P<values>[0-9,.]+)")
-
-        regex_results = version_re.search(revision_info['subject'])
-
-        if regex_results:
-          version = regex_results.group('values')
-
-          self.ChangeToDepotWorkingDirectory('v8_bleeding_edge')
-
-          git_revision = self.source_control.ResolveToRevision(
-              int(svn_revision) - 1, 'v8_bleeding_edge', -1)
-
-          if git_revision:
-            revision_info = self.source_control.QueryRevisionInfo(git_revision)
-
-            if 'Prepare push to trunk' in revision_info['subject']:
-              results['v8_bleeding_edge'] = git_revision
-
     return results
 
   def BuildCurrentRevision(self, depot):
@@ -1607,29 +1640,50 @@ class BisectPerformanceMetrics(object):
 
     return dist_to_good_value < dist_to_bad_value
 
-  def ChangeToDepotWorkingDirectory(self, depot_name):
-    """Given a depot, changes to the appropriate working directory.
-
-    Args:
-      depot_name: The name of the depot (see DEPOT_NAMES).
-    """
+  def _GetDepotDirectory(self, depot_name):
     if depot_name == 'chromium':
-      os.chdir(self.src_cwd)
+      return self.src_cwd
     elif depot_name == 'cros':
-      os.chdir(self.cros_cwd)
+      return self.cros_cwd
     elif depot_name in DEPOT_NAMES:
-      os.chdir(self.depot_cwd[depot_name])
+      return self.depot_cwd[depot_name]
     else:
       assert False, 'Unknown depot [ %s ] encountered. Possibly a new one'\
                     ' was added without proper support?' %\
                     (depot_name,)
 
-  def FindNextDepotToBisect(self, current_revision, min_revision_data,
-      max_revision_data):
+  def ChangeToDepotWorkingDirectory(self, depot_name):
+    """Given a depot, changes to the appropriate working directory.
+
+    Args:
+      depot_name: The name of the depot (see DEPOT_NAMES).
+    """
+    os.chdir(self._GetDepotDirectory(depot_name))
+
+  def _FillInV8BleedingEdgeInfo(self, min_revision_data, max_revision_data):
+    r1 = self._GetNearestV8BleedingEdgeFromTrunk(min_revision_data['revision'],
+        search_forward=True)
+    r2 = self._GetNearestV8…
(message too large)

[commit-bot: I haz the power, 2013-11-04 19:41:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/50043003/70001

[commit-bot: I haz the power, 2013-11-04 22:44:43]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-11-04 23:32:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/50043003/70001

[commit-bot: I haz the power, 2013-11-05 00:54:47]

Change committed as 232859