Bug 225616

Summary: [webkitscmpy] Cache identifiers in Git checkouts
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Dean Johnson <dean_johnson>
Status: RESOLVED FIXED    
Severity: Normal CC: dean_johnson, dewei_zhu, ews-watchlist, glenn, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=226891
https://bugs.webkit.org/show_bug.cgi?id=225889
https://bugs.webkit.org/show_bug.cgi?id=227082
https://bugs.webkit.org/show_bug.cgi?id=227150
https://bugs.webkit.org/show_bug.cgi?id=227313
https://bugs.webkit.org/show_bug.cgi?id=227327
https://bugs.webkit.org/show_bug.cgi?id=228000
https://bugs.webkit.org/show_bug.cgi?id=228027
Bug Depends on:    
Bug Blocks: 225986    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Add identifiers cache
none
Add identifiers cache v2
none
Identifiers cache v2
none
Patch
none
Patch
none
Patch for landing
none
Patch
dewei_zhu: review+
Patch jbedard: commit-queue+

Jonathan Bedard
Reported 2021-05-10 14:36:43 PDT
We need to create an identifier cache fast enough for use by `git log` and `git blame`
Attachments
Patch (15.46 KB, patch)
2021-05-10 14:47 PDT, Jonathan Bedard
no flags
Patch (15.40 KB, patch)
2021-05-11 08:28 PDT, Jonathan Bedard
no flags
Patch (22.94 KB, patch)
2021-05-11 16:31 PDT, Jonathan Bedard
no flags
Add identifiers cache (48.98 KB, patch)
2021-05-20 13:53 PDT, Dean Johnson
no flags
Add identifiers cache v2 (46.74 KB, patch)
2021-05-20 13:58 PDT, Dean Johnson
no flags
Identifiers cache v2 (46.74 KB, patch)
2021-05-20 14:00 PDT, Dean Johnson
no flags
Patch (34.99 KB, patch)
2021-06-28 14:29 PDT, Jonathan Bedard
no flags
Patch (28.00 KB, patch)
2021-06-29 14:12 PDT, Jonathan Bedard
no flags
Patch for landing (28.04 KB, patch)
2021-06-30 16:26 PDT, Jonathan Bedard
no flags
Patch (1.91 KB, patch)
2021-07-01 15:13 PDT, Jonathan Bedard
dewei_zhu: review+
Patch (3.32 KB, patch)
2021-07-01 15:24 PDT, Jonathan Bedard
jbedard: commit-queue+
Radar WebKit Bug Importer
Comment 1 2021-05-10 14:37:00 PDT
Jonathan Bedard
Comment 2 2021-05-10 14:47:24 PDT
Jonathan Bedard
Comment 3 2021-05-11 08:28:17 PDT
Jonathan Bedard
Comment 4 2021-05-11 16:31:34 PDT
Dean Johnson
Comment 5 2021-05-20 13:31:16 PDT
Taking this from Jonathan since we've been iterating on this together and I've got a new patch to upload.
Dean Johnson
Comment 6 2021-05-20 13:53:40 PDT
Created attachment 429213 [details] Add identifiers cache
Dean Johnson
Comment 7 2021-05-20 13:58:41 PDT
Created attachment 429215 [details] Add identifiers cache v2 Fixed a couple test failures caused by refactoring. Still need to figure out how to correctly handle remotes for these tests: [1806/1995] webkitscmpy.test.canonicalize_unittest.TestCanonicalize.test_branch_commits failed: Traceback (most recent call last): File "/Volumes/Data/dean-home/Projects/Safari/OpenSource/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/canonicalize_unittest.py", line 199, in test_branch_commits self.assertEqual(commit_a.message, 'New commit 1\nIdentifier: 2.3@branch-a') AssertionError: 'New commit 1\nIdentifier: 2.4@branch-a' != 'New commit 1\nIdentifier: 2.3@branch-a' New commit 1 - Identifier: 2.4@branch-a? ^ + Identifier: 2.3@branch-a? ^ [1823/1995] webkitscmpy.test.checkout_unittest.TestCheckout.test_checkout_git_svn failed: Traceback (most recent call last): File "/Volumes/Data/dean-home/Projects/Safari/OpenSource/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/checkout_unittest.py", line 62, in test_checkout_git_svn self.assertEqual('621652add7fc416099bd2063366cc38ff61afe36', local.Git(self.path).commit().hash) AssertionError: '621652add7fc416099bd2063366cc38ff61afe36' != 'fff83bb2d9171b4d9196e977eb0508fd57e7a08d' - 621652add7fc416099bd2063366cc38ff61afe36 + fff83bb2d9171b4d9196e977eb0508fd57e7a08d And how to handle the broken SVN detection test: [153/1995] webkitpy.common.checkout.scm.detection_unittest.SCMDetectorTest.test_detect_scm_system failed: Traceback (most recent call last): File "/Volumes/Data/dean-home/Projects/Safari/OpenSource/Tools/Scripts/webkitpy/common/checkout/scm/detection_unittest.py", line 49, in test_detect_scm_system self.assertEqual( AssertionError: "MOCK run_command: ['git', 'rev-parse', '--is-inside-work-tree'], cwd=/\n" != "MOCK run_command: ['svn', 'info'], cwd=/\nMOCK run_command: [[49 chars]=/\n" + MOCK run_command: ['svn', 'info'], cwd=/ MOCK run_command: ['git', 'rev-parse', '--is-inside-work-tree'], cwd=/
Dean Johnson
Comment 8 2021-05-20 14:00:36 PDT
Created attachment 429217 [details] Identifiers cache v2
Jonathan Bedard
Comment 9 2021-06-28 14:29:41 PDT
Jonathan Bedard
Comment 10 2021-06-28 15:33:10 PDT
Comment on attachment 432420 [details] Patch A high-level comment: This ends up being a 3-4x improvement on `git-webkit` commands in most cases. Where it's really important, though, is in `git log` and `git blame`, which will come in a later patch. Without this cache, implementations of those two commands would be extremely slow
Jonathan Bedard
Comment 11 2021-06-29 14:12:07 PDT
dewei_zhu
Comment 12 2021-06-30 15:19:02 PDT
Comment on attachment 432528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432528&action=review r=me > Tools/Scripts/libraries/webkitscmpy/webkitscmpy/local/git.py:51 > + if os.path.exists(self.path): Maybe we can do `` if not os.path.exists(self.path): return `` to avoid indentation.
Jonathan Bedard
Comment 13 2021-06-30 16:26:15 PDT
Created attachment 432642 [details] Patch for landing
EWS
Comment 14 2021-06-30 17:16:25 PDT
Committed r279445 (239301@main): <https://commits.webkit.org/239301@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432642 [details].
Jonathan Bedard
Comment 15 2021-07-01 15:13:24 PDT
Reopening to attach new patch.
Jonathan Bedard
Comment 16 2021-07-01 15:13:25 PDT
dewei_zhu
Comment 17 2021-07-01 15:17:06 PDT
Comment on attachment 432729 [details] Patch r=me
Jonathan Bedard
Comment 18 2021-07-01 15:24:33 PDT
Jonathan Bedard
Comment 19 2021-07-01 17:20:57 PDT
Note You need to log in before you can comment on or make changes to this bug.