Repository navigation
fix(workingdiff): diff committed trees with one git process - #71
Merged
Merged
Conversation
Observing a commit or branch target walked both endpoint trees with one `git cat-file tree` process per directory, so cost grew with repository size and moderately sized repositories exceeded the 8 s observation budget. Compare the trees with a single `git diff-tree` that returns only changed entries, bracketed by the object store authority check used for merge-base, and use the same path-limited comparison when revalidating a file on read. Fixes #68
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.
Fixes #68.
Problem
Observing a commit or branch diff target read both endpoint trees in full, starting one
git cat-file treeprocess per directory. Cost scaled with repository size rather than change size: in this repository (306 directories) two walks took ~8.4 s against the 8 s observation budget, so every commit target failed withcontext deadline exceeded. Reading a file from a committed observation re-walked both full trees as well.Change
diffCommittedTreesruns a singlegit diff-tree -r -z --raw --no-renames --ignore-submodules=none <base> <head>and returns only changed entries. The empty-tree base uses git's well-known empty tree OID for the repository's object format.validateObjectStoreAuthority, the same before/after loose-object and pack checkmergeBaseuses, so unsafe object storage is still rejected.revalidateCommittedFile(file reads) uses the same comparison restricted to the requested path.Behavior notes
diff-treeoutput is trusted, asls-treealready is for working-tree observation.repository_unavailablerather thanlimit_exceeded.Validation
diff-treeand at most 10 git processes, counted by agitwrapper onPATH. The test fails onmainwithcontext deadline exceeded.gofmt -l .,go build ./...,go vet ./...,go test ./...,go test -race ./internal/workingdiff/.#69 and #70 (error propagation for diff failures) are not addressed here.