Changeset 70059 in webkit

Timestamp:

Oct 19, 2010 8:21:01 AM (14 years ago)

Author:

abarth@webkit.org

Message:

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:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/checkout/api.py (modified) (1 diff)
• Scripts/webkitpy/common/checkout/scm.py (modified) (3 diffs)
• Scripts/webkitpy/tool/mocktool.py (modified) (2 diffs)
• Scripts/webkitpy/tool/steps/abstractstep.py (modified) (2 diffs)
• Scripts/webkitpy/tool/steps/editchangelog.py (modified) (1 diff)
• Scripts/webkitpy/tool/steps/preparechangelog.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+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>
 

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

@@ -84 +84 @@
         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):

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

@@ -261 +261 @@
         self._subclass_must_implement()
 
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         self._subclass_must_implement()
 
@@ -458 +458 @@
 
     # 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)
@@ -690 +690 @@
         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

trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

@@ -430 +430 @@
         self.checkout_root = self.fake_checkout_root
 
-    def create_patch(self, git_commit):
+    def create_patch(self, git_commit, changed_files=None):
         return "Patch1"
 
@@ -476 +476 @@
         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!

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

@@ -46 +46 @@
         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)),
     }
 
@@ -60 +64 @@
         return state[key]
 
+    def did_modify_checkout(self, state):
+        state["diff"] = None
+        state["changelogs"] = None
+        state["changed_files"] = None
+
     @classmethod
     def options(cls):

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

@@ -36 +36 @@
         os.chdir(self._tool.scm().checkout_root)
         self._tool.user.edit_changelog(self.cached_lookup(state, "changelogs"))
+        self.did_modify_checkout(state)

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

@@ -75 +75 @@
         except ScriptError, e:
             error("Unable to prepare ChangeLogs.")
-        state["diff"] = None # We've changed the diff
+        self.did_modify_checkout(state)