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+

Description Jonathan Bedard 2021-05-10 14:36:43 PDT
We need to create an identifier cache fast enough for use by `git log` and `git blame`
Comment 1 Radar WebKit Bug Importer 2021-05-10 14:37:00 PDT
<rdar://problem/77789230>
Comment 2 Jonathan Bedard 2021-05-10 14:47:24 PDT
Created attachment 428204 [details]
Patch
Comment 3 Jonathan Bedard 2021-05-11 08:28:17 PDT
Created attachment 428279 [details]
Patch
Comment 4 Jonathan Bedard 2021-05-11 16:31:34 PDT
Created attachment 428327 [details]
Patch
Comment 5 Dean Johnson 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.
Comment 6 Dean Johnson 2021-05-20 13:53:40 PDT
Created attachment 429213 [details]
Add identifiers cache
Comment 7 Dean Johnson 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=/
Comment 8 Dean Johnson 2021-05-20 14:00:36 PDT
Created attachment 429217 [details]
Identifiers cache v2
Comment 9 Jonathan Bedard 2021-06-28 14:29:41 PDT
Created attachment 432420 [details]
Patch
Comment 10 Jonathan Bedard 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
Comment 11 Jonathan Bedard 2021-06-29 14:12:07 PDT
Created attachment 432528 [details]
Patch
Comment 12 dewei_zhu 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.
Comment 13 Jonathan Bedard 2021-06-30 16:26:15 PDT
Created attachment 432642 [details]
Patch for landing
Comment 14 EWS 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].
Comment 15 Jonathan Bedard 2021-07-01 15:13:24 PDT
Reopening to attach new patch.
Comment 16 Jonathan Bedard 2021-07-01 15:13:25 PDT
Created attachment 432729 [details]
Patch
Comment 17 dewei_zhu 2021-07-01 15:17:06 PDT
Comment on attachment 432729 [details]
Patch

r=me
Comment 18 Jonathan Bedard 2021-07-01 15:24:33 PDT
Created attachment 432732 [details]
Patch
Comment 19 Jonathan Bedard 2021-07-01 17:20:57 PDT
Committed r279488 (239340@main): <https://commits.webkit.org/239340@main>