storage: filesystem, pool FSObject reads via PackHandle#2154
Merged
Conversation
hiddeco
force-pushed
the
fix-fsobject-shared-packhandle
branch
2 times, most recently
from
226f97c to
d69e5a5
Compare
pjbgf
reviewed
May 26, 2026
pjbgf
reviewed
May 26, 2026
hiddeco
force-pushed
the
fix-fsobject-shared-packhandle
branch
2 times, most recently
from
50a8b1f to
d1cecf0
Compare
Materialising an `FSObject` returned by `EncodedObject` previously fell back to a direct `o.fs.Open(packPath)` on every `Reader()` call, bypassing the per-pack file-descriptor pool introduced in `billy.Filesystem` that pays an mmap/munmap on each Open, the regression amplified to ~400x more pack opens over 500 reads on the basic fixture[1]. The cure threads the dotgit-cached `*packhandle.PackHandle` into the `*packfile.Packfile` via a new `WithPackHandle` option taking a `PackHandleResolver`. `objectFromHeader` captures the resolver in the `FSObject.acquireRandom` closure, so every read acquires a fresh cursor against the pool-owned `SharedFile`. The resolver re-runs on every `Reader` call, which lets cached FSObjects survive resets of dotgit's pack-handle store such as the one `NewObjectPack` triggers before a repack. The internal `*packhandle.PackHandle` stays internal — the public `packfile.PackHandle` interface returns `io.ReadSeekCloser` and `RandomReader`, and `DotGit.PackHandle` returns the interface through an unexported adapter so callers can't accidentally call `Close()` on a dotgit-owned handle. `Packfile.Close` releases only the scanner cursor; the resolver-owned handle outlives the Packfile. [1]: go-git#2153 (comment) Assisted-by: Claude Opus 4.7 Signed-off-by: Hidde Beydals <[email protected]>
hiddeco
force-pushed
the
fix-fsobject-shared-packhandle
branch
from
d1cecf0 to
2a435ca
Compare
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
2 participants
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.
Materialising an
FSObjectreturned byEncodedObjectpreviously fell back to a directo.fs.Open(packPath)on everyReader()call, bypassing the per-pack file-descriptor pool introduced inbilly.Filesystemthat pays an mmap/munmap on each Open, the regression amplified to ~400x more pack opens over 500 reads on the basic fixture1.The cure threads the dotgit-cached
*packhandle.PackHandleinto the*packfile.Packfilevia a newWithPackHandleoption taking aPackHandleResolver.objectFromHeadercaptures the resolver in theFSObject.acquireRandomclosure, so every read acquires a fresh cursor against the pool-ownedSharedFile. The resolver re-runs on everyReadercall, which lets cached FSObjects survivecleanPackListinvalidations such as the oneNewObjectPacktriggers before a repack.The internal
*packhandle.PackHandlestays internal — the publicpackfile.PackHandleinterface returnsio.ReadSeekCloserandRandomReader, andDotGit.PackHandlereturns the interface through an unexported adapter so callers can't accidentally callClose()on a catalog-owned handle.Packfile.Closejoins the scanner-close and owned-handle close errors viaerrors.Join.