2010-10-19 Adam Barth <abarth@webkit.org>

        Reviewed by Eric Seidel.

        webkit-patch stats the filesystem too many times
        https://bugs.webkit.org/show_bug.cgi?id=47883

        This patch attempts to cache the list of changed files more agressively
        and to use that list to compute the diff instead of stating the file
        system again.

        * Scripts/webkitpy/common/checkout/api.py:
        * Scripts/webkitpy/common/checkout/scm.py:
        * Scripts/webkitpy/tool/mocktool.py:
        * Scripts/webkitpy/tool/steps/abstractstep.py:
        * Scripts/webkitpy/tool/steps/editchangelog.py:
        * Scripts/webkitpy/tool/steps/preparechangelog.py:

Canonical link: https://commits.webkit.org/60621@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@70059 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog

@@ -1,3 +1,21 @@
+2010-10-19  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        webkit-patch stats the filesystem too many times
+        https://bugs.webkit.org/show_bug.cgi?id=47883
+
+        This patch attempts to cache the list of changed files more agressively
+        and to use that list to compute the diff instead of stating the file
+        system again.
+
+        * Scripts/webkitpy/common/checkout/api.py:
+        * Scripts/webkitpy/common/checkout/scm.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+        * Scripts/webkitpy/tool/steps/abstractstep.py:
+        * Scripts/webkitpy/tool/steps/editchangelog.py:
+        * Scripts/webkitpy/tool/steps/preparechangelog.py:
+
 2010-10-19  David Kilzer  <ddkilzer@apple.com>
 
         <http://webkit.org/b/47741> Make sort-Xcode-project-file a little more friendly

WebKitTools/Scripts/webkitpy/common/checkout/api.py

@@ -83,19 +83,20 @@ def commit_info_for_revision(self, revision):
     def bug_id_for_revision(self, revision):
         return self.commit_info_for_revision(revision).bug_id()
 
-    def _modified_files_matching_predicate(self, git_commit, predicate):
+    def _modified_files_matching_predicate(self, git_commit, predicate, changed_files=None):
         # SCM returns paths relative to scm.checkout_root
         # Callers (especially those using the ChangeLog class) may
         # expect absolute paths, so this method returns absolute paths.
-        changed_files = self._scm.changed_files(git_commit)
+        if not changed_files:
+            changed_files = self._scm.changed_files(git_commit)
         absolute_paths = [os.path.join(self._scm.checkout_root, path) for path in changed_files]
         return [path for path in absolute_paths if predicate(path)]
 
-    def modified_changelogs(self, git_commit):
-        return self._modified_files_matching_predicate(git_commit, self._is_path_to_changelog)
+    def modified_changelogs(self, git_commit, changed_files=None):
+        return self._modified_files_matching_predicate(git_commit, self._is_path_to_changelog, changed_files=changed_files)
 
-    def modified_non_changelogs(self, git_commit):
-        return self._modified_files_matching_predicate(git_commit, lambda path: not self._is_path_to_changelog(path))
+    def modified_non_changelogs(self, git_commit, changed_files=None):
+        return self._modified_files_matching_predicate(git_commit, lambda path: not self._is_path_to_changelog(path), changed_files=changed_files)
 
     def commit_message_for_this_commit(self, git_commit):
         changelog_paths = self.modified_changelogs(git_commit)

WebKitTools/Scripts/webkitpy/common/checkout/scm.py

@@ -260,7 +260,7 @@ def conflicted_files(self):
     def display_name(self):
         self._subclass_must_implement()
 
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         self._subclass_must_implement()
 
     def committer_email_for_revision(self, revision):
@@ -457,11 +457,11 @@ def display_name(self):
         return "svn"
 
     # FIXME: This method should be on Checkout.
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         """Returns a byte array (str()) representing the patch file.
         Patch files are effectively binary since they may contain
         files of multiple different encodings."""
-        return self.run([self.script_path("svn-create-patch")],
+        return self.run([self.script_path("svn-create-patch")] + changed_files,
             cwd=self.checkout_root, return_stderr=False,
             decode_output=False)
 
@@ -689,12 +689,12 @@ def supports_local_commits():
     def display_name(self):
         return "git"
 
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         """Returns a byte array (str()) representing the patch file.
         Patch files are effectively binary since they may contain
         files of multiple different encodings."""
         # FIXME: This should probably use cwd=self.checkout_root
-        return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit)], decode_output=False)
+        return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit)] + changed_files, decode_output=False)
 
     @classmethod
     def git_commit_from_svn_revision(cls, revision):

WebKitTools/Scripts/webkitpy/tool/mocktool.py

@@ -429,7 +429,7 @@ def __init__(self):
         # will actually be the root.  Since getcwd() is wrong, use a globally fake root for now.
         self.checkout_root = self.fake_checkout_root
 
-    def create_patch(self, git_commit):
+    def create_patch(self, git_commit, changed_files=None):
         return "Patch1"
 
     def commit_ids_from_commitish_arguments(self, args):
@@ -475,7 +475,7 @@ def bug_id_for_revision(self, svn_revision):
     def recent_commit_infos_for_files(self, paths):
         return [self.commit_info_for_revision(32)]
 
-    def modified_changelogs(self, git_commit):
+    def modified_changelogs(self, git_commit, changed_files=None):
         # Ideally we'd return something more interesting here.  The problem is
         # that LandDiff will try to actually read the patch from disk!
         return []

WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py

@@ -45,10 +45,14 @@ def _run_script(self, script_name, args=None, quiet=False, port=WebKitPort):
             command.extend(args)
         self._tool.executive.run_and_throw_if_fail(command, quiet)
 
+    def _changed_files(self, state):
+        return self.cached_lookup(state, "changed_files")
+
     _well_known_keys = {
-        "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit),
-        "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit),
         "bug_title": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]).title(),
+        "changed_files": lambda self, state: self._tool.scm().changed_files(self._options.git_commit),
+        "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit, changed_files=self._changed_files(state)),
+        "changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit, changed_files=self._changed_files(state)),
     }
 
     def cached_lookup(self, state, key, promise=None):
@@ -59,6 +63,11 @@ def cached_lookup(self, state, key, promise=None):
         state[key] = promise(self, state)
         return state[key]
 
+    def did_modify_checkout(self, state):
+        state["diff"] = None
+        state["changelogs"] = None
+        state["changed_files"] = None
+
     @classmethod
     def options(cls):
         return [

WebKitTools/Scripts/webkitpy/tool/steps/editchangelog.py

@@ -35,3 +35,4 @@ class EditChangeLog(AbstractStep):
     def run(self, state):
         os.chdir(self._tool.scm().checkout_root)
         self._tool.user.edit_changelog(self.cached_lookup(state, "changelogs"))
+        self.did_modify_checkout(state)

WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py

@@ -74,4 +74,4 @@ def run(self, state):
             self._tool.executive.run_and_throw_if_fail(args, self._options.quiet)
         except ScriptError, e:
             error("Unable to prepare ChangeLogs.")
-        state["diff"] = None # We've changed the diff
+        self.did_modify_checkout(state)