Repository navigation
git: blame, Avoid repeated tree and blob lookups - #2585
Open
Muhammad-Bin-Ali wants to merge 6 commits into
Open
Muhammad-Bin-Ali wants to merge 6 commits into
Muhammad-Bin-Ali wants to merge 6 commits into
Conversation
There was no benchmark for `Blame`, so there was no way to measure changes to it. `BenchmarkBlame` blames five fixture files, each chosen to stress a different part of the history walk: a long script, a binary file, the most edited file in a repository, a renamed file and a file left unchanged for most of the history. The fixture packfiles are written to a repository on disk, as `Blame` is used on a cloned repository. Each file is benchmarked cold, with a new storage per iteration and the read of the starting commit timed along with `Blame`, so loading the pack indexes counts towards the result, and warm, with one storage reused across iterations. Assisted-by: Claude Opus 5.5 Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
Timings vary between runs and machines, which makes it hard to tell why a change helped. `BenchmarkBlame` now also reports the number of commits, trees and blobs a `Blame` call reads, as `commits/op`, `trees/op` and `blobs/op`. They are counted once per file by wrapping the storage in a `countingObjectStorer`, so the timed loops read from the storage directly. Assisted-by: Claude Opus 5.5 Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
The fixtures have short histories, so `BenchmarkBlame` cannot show how `Blame` behaves on a real project. `BenchmarkBlameRepository` blames files of a repository on disk, set through environment variables: GOGIT_BLAME_BENCH_REPO path to the repository GOGIT_BLAME_BENCH_FILES comma-separated files to blame GOGIT_BLAME_BENCH_REV revision to blame at, HEAD by default It is skipped when `GOGIT_BLAME_BENCH_REPO` is not set, so it never runs in CI. It shares the cold and warm runs and object counts of `BenchmarkBlame`. Assisted-by: Claude Opus 5.5 Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
For each parent of each commit it visits, `Blame` looked up the blob of the blamed file in that commit to compare it with the parent's. Every lookup decoded the commit's trees from the root and read the blob, although the blob cannot change once the commit is queued: it is the file `Blame` started from, or the parent's blob it was compared with when the commit was queued. Keep that hash on the queued item and compare against it, instead of looking it up again. With `BenchmarkBlame`, and `BenchmarkBlameRepository` blaming files of this repository at 12b788e, `Blame` reads 29-33% fewer trees and 18-33% fewer blobs, and warm runs take 11-24% less time on an Apple M3 Pro: trees/op blobs/op warm go-git-unchanged 1334 -> 898 699 -> 481 -20% blame.go 15.77k -> 10.56k 15.84k -> 10.62k -20% object/commit.go 45.56k -> 30.56k 16.22k -> 10.98k -24% Add tests for merges and for an edit that is later reverted. The latter fails if the child's hash is queued in place of the parent's. Assisted-by: Claude Opus 5.5 Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
`Blame` looked up the blamed path in each parent up to three times:
`parentsContainingPath` read it to check that the parent has the
file, `blobHash` read it again for its hash, and the contents were
read a third time when the hash differed. Each lookup decoded the
parent's trees from the root and read the blob.
Look the path up once in the parent's tree. A parent has the file
when the path names a blob in it, as Git's `fill_blob_sha1_and_mode`
requires [1]. An entry with the child's hash names the child's blob,
which `Blame` has already read as a blob, so the parent's blob is only
read when its hash differs, and that read gives the contents the diff
needs.
Renamed files are read from the same tree, at their old path.
The result is unchanged. In particular, a directory or a submodule at
the path still sends `Blame` to rename detection, as reading it as a
blob fails.
Measured as for the previous commit, `Blame` reads 46-55% fewer trees
and 43-98% fewer blobs than after it, and warm runs take 16-40% less
time:
trees/op blobs/op warm
go-git-unchanged 898 -> 462 481 -> 45 -29%
blame.go 10.56k -> 5.25k 10.62k -> 194 -35%
object/commit.go 30.56k -> 15.05k 10.98k -> ~514 -40%
Some blob counts vary by a few between runs, as they did before this
change; the table shows medians.
[1]: https://github.com/git/git/blob/v2.47.1/blame.c#L1240-L1254
Assisted-by: Claude Opus 5.5
Signed-off-by: Muhammad Bin Ali <muhammadbinali706@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #14. Depends on #2571, which adds the benchmarks; only the last two commits are new here. I'll rebase onto
mainonce #2571 merges.Why
For each parent of each commit it visited,
Blamelooked the blamed file up once in the commit and up to three times in the parent. Each lookup decoded the trees from the root and read the blob, often only for its hash. In a profile ofgo-git-unchanged, about three quarters of the CPU time underBlamewas inEncodedObject, mostly pack-index lookups that every read pays, even on a cache hit.What + Future Intention
fill_blob_sha1_and_mode. The parent's blob is read only when its hash differs from the child's, which is also when the diff needs its contents.Blame's output is unchanged. Follow-ups: #2584 (type changes, which this PR leaves as they are), rename detection without building full patches, and Bloom filters (#2529).Verification
Benchmarks ran on an Apple M3 Pro, with
mainand both commits run in alternating rounds.BenchmarkBlameRepositoryblames this repository at 12b788e.go-git-unchangedblame.goplumbing/object/commit.goAcross all cases, warm runs are 25–55% faster and cold runs 10–35% faster. The number of commits read is unchanged.
benchstat: main vs this PR
TestBlameSuite, covering merges, a reverted edit, a file replacing or moved over a directory, a mode change and renames. Each expected result matchesgit blame, and I checked that each test fails when the mistake it targets is applied.Blame's output is byte-identical before and after each commit on 14 files: 9 from this repository, including renamed files, and the 5 benchmark fixtures.go testpasses for the root package and./plumbing/object/...,TestBlameSuitepasses with-race, andgolangci-lintreports 0 issues.AI assistance
Developed with Claude Code (Claude Opus 5.5), which was used to profile
Blame, write the changes and tests, and run the verification. The commits carryAssisted-bytrailers.🤖 Generated with Claude Code