Bug 225616 - [webkitscmpy] Cache identifiers in Git checkouts
Summary: [webkitscmpy] Cache identifiers in Git checkouts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Johnson
URL:
Keywords: InRadar
Depends on:
Blocks: 225986
  Show dependency treegraph
 
Reported: 2021-05-10 14:36 PDT by Jonathan Bedard
Modified: 2021-07-16 10:19 PDT (History)
5 users (show)

See Also:


Attachments
Patch (15.46 KB, patch)
2021-05-10 14:47 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (15.40 KB, patch)
2021-05-11 08:28 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (22.94 KB, patch)
2021-05-11 16:31 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Add identifiers cache (48.98 KB, patch)
2021-05-20 13:53 PDT, Dean Johnson
no flags Details | Formatted Diff | Diff
Add identifiers cache v2 (46.74 KB, patch)
2021-05-20 13:58 PDT, Dean Johnson
no flags Details | Formatted Diff | Diff
Identifiers cache v2 (46.74 KB, patch)
2021-05-20 14:00 PDT, Dean Johnson
no flags Details | Formatted Diff | Diff
Patch (34.99 KB, patch)
2021-06-28 14:29 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (28.00 KB, patch)
2021-06-29 14:12 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (28.04 KB, patch)
2021-06-30 16:26 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (1.91 KB, patch)
2021-07-01 15:13 PDT, Jonathan Bedard
dewei_zhu: review+
Details | Formatted Diff | Diff
Patch (3.32 KB, patch)
2021-07-01 15:24 PDT, Jonathan Bedard
jbedard: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>