odb: scan all sources' packfiles before loose objects - #975
Merged
Johannes Schindelin (dscho) merged 1 commit intoAug 10, 2026
Merged
Johannes Schindelin (dscho) merged 1 commit into
Johannes Schindelin (dscho) merged 1 commit into
Conversation
The object database refactor that introduced per-source object stores (cb506a8 "odb: introduce \"files\" source" and the surrounding series, first released in v2.54.0) changed how do_oid_object_info_extended() searches for an object. It now iterates the sources and, within each source, consults that source's packfiles and then its loose object store before moving on to the next source. Before that series the search consulted every packfile -- across the primary object directory and all alternates -- before it looked at any loose object. The refactor reversed that for the multi-source case: for an object that lives in an alternate's packfile, the primary source's loose object store is now consulted first. That loose lookup is a filesystem stat(), and because callers such as cache_tree_fully_valid() pass ODB_HAS_OBJECT_RECHECK_PACKED (which clears OBJECT_INFO_QUICK) the cached-loose-index fast path is skipped and a real stat() runs for every such object. In a repository that keeps its objects in an alternate -- the common arrangement for VFS for Git and Scalar enlistments, where a shared object cache is mounted as an alternate -- this is a steep penalty. cache_tree_fully_valid() walks the whole cache tree and calls odb_has_object() for every node; on an enlistment with a ~2.4M-entry index that is ~380k objects, each incurring a wasted stat() on the primary loose store. A same-commit branch switch spent ~32s in cache_tree_fully_valid() (two calls of ~16s), observed in the field as a ~2x rise in median checkout duration after the client carrying the refactor rolled out. Restore the previous ordering without undoing the per-source encapsulation: when there is more than one source, scan the packfiles of every source first (OBJECT_INFO_SKIP_LOOSE) and only then consult each source's loose store (OBJECT_INFO_SKIP_PACKED). The single-source case is unchanged, so repositories without alternates keep the existing path. With the fix the same branch switch spends ~2s in cache_tree_fully_valid(), the ~380k wasted stat()s are gone, and performance matches versions predating the refactor. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Tyrie Vella (tyrielv)
marked this pull request as ready for review
August 7, 2026 22:18
Johannes Schindelin (dscho)
approved these changes
Aug 10, 2026
Johannes Schindelin (dscho)
left a comment
Johannes Schindelin (dscho)
left a comment
Member
There was a problem hiding this comment.
I analyzed this thoroughly, and while the fix should probably live at a slightly different layer (do_oid_object_info_extended() is used by more callers than odb_has_object()), it is defendable to merge this as a critical fix to reinstate a better developer experience of VFS for Git users.
The upstream changes that are planned to address this issue will look very different: The idea is to move alternates handling into the files backend, and then still keep the packfiles-before-loose-objects order, albeit now once again taking all packfiles of primary and alternates into account.
Naturally, that fix will require two or three preparatory refactoring patch series, and hence the timeline of that fix materializing is prohibitively far in the future. Therefore it's better to take this as-is, for now, and probably just revert it once the upstream fixes are tried-and-proven.
Johannes Schindelin (dscho)
merged commit Aug 10, 2026
f7ebbb5
into
microsoft:vfs-2.55.0
93 checks passed
Johannes Schindelin (dscho)
added a commit
that referenced
this pull request
Sep 22, 2026
Switching branches in a VFS for Git or Scalar enlistment became drastically slower in Git v2.55. The cause is the interaction of two independent changes, not a single one. Commit 062b914 (treewide: convert users of repo_has_object_file() to has_object()), first released in v2.50, accidentally inverted the object existence check in cache_tree_fully_valid(), so that function bailed out at the root instead of validating the cache tree recursively. Commit 5217312 (cache-tree: fix inverted object existence check in cache_tree_fully_valid), first released in v2.55, correctly restored the recursion. In between, v2.54's per-source object database refactor changed the cross-source lookup order: instead of scanning all packfiles before any loose object store, it scans packed then loose per source. That change caused no observable checkout regression in v2.54 precisely because the inverted check prevented recursion and thereby masked the per-object cost. Conversely, v2.49 did recurse, yet stayed fast because its global pack-first lookup found the objects in an alternate's packs before attempting any loose lookup. v2.55 is therefore the first version combining recursive cache-tree validation with per-source packed-then-loose lookup, at a cost of roughly one wasted primary loose-object lstat() per cache-tree node. In enlistments that use an alternate object cache, that cost is severe. cache_tree_fully_valid() calls odb_has_object() hundreds of thousands of times, and ODB_HAS_OBJECT_RECHECK_PACKED clears OBJECT_INFO_QUICK, so each call performs a real lstat() in the primary loose object store before the object is found in the alternate's packfile. On a measured index with about 2.4M entries: 381,006 cache-tree nodes, 380,944 wasted lstat() calls, not a single miss; cache-tree validation took about 32 seconds and switching to a branch pointing at the same commit about 36 seconds. This merge restores the all-sources-packed-before-all-sources-loose order whenever more than one source is present. The wasted stats are gone, validation drops to about 2 seconds and branch switching to about 6-7 seconds. Presence semantics are unchanged. The trade-off needs to be stated plainly: the mitigation lives in the shared object-info lookup, which is a slightly incorrect representation layer for a problem specific to presence-only queries via odb_has_object(). When an object is loose in the primary and packed in an alternate, metadata callers now observe the alternate's packed representation. Observable differences include reported on-disk size, mtime, delta base, corruption handling, and promisor-pack classification. Object content, type and logical size, as well as presence, remain correct because objects are content-addressed, which bounds the fallout to the representation level. We accept those behavior changes deliberately in exchange for fixing an intolerable regression in this fork now. The architecturally correct fix is upstream's plan to move alternate handling into the files backend. That backend would own both the primary and the alternates and could therefore scan all relevant packs before any loose-object lookup without violating the abstraction. The upstream contributor estimates that work at "three to four patch years", i.e. likely months in Git project time, and probably not before Git 2.56. microsoft/git cannot wait that long. This merge is consequently an explicitly temporary mitigation specific to Microsoft Git. Once upstream's fix lands, this implementation should be replaced and the regression test reassessed. That test currently asserts packed-versus-loose selection through %(deltabase), which necessarily pins representation ordering and may not survive the upstream architecture. A future replacement could instead verify the absence of the unwanted lstat() calls directly, possibly as a Linux-only strace test, since the behavior itself is platform-independent.
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.
Problem
Since v2.55,
git checkout(and any branch-changing operation) isdramatically slower on large VFS for Git / Scalar enlistments. A
same-commit branch switch on a ~2.4M-entry index went from under a
second to ~36s. Fixes #974.
Root cause
The upstream per-source object database refactor (first released in
v2.54.0) changed object lookup from "scan all packfiles, then all loose
objects" to "per source: packed then loose." For an object that lives in
an alternate's packfile -- the normal arrangement for VFS for Git and
Scalar enlistments, where a shared object cache is mounted as an
alternate -- the primary source's loose object store is now consulted
first. That loose lookup is a filesystem
stat(), and because callerssuch as
cache_tree_fully_valid()passODB_HAS_OBJECT_RECHECK_PACKED(which clears
OBJECT_INFO_QUICK) the cached-loose-index fast path isskipped, so a real
stat()runs for every object.cache_tree_fully_valid()walks the whole cache tree and callsodb_has_object()for every node -- ~380k objects on this index -- eachincurring a wasted
stat(). Full analysis, the instrumented breakdown(
loose_lstats: 380,944,odb_misses: 0), and the alternativesconsidered are in #974.
Fix
Preserve the refactor's per-source encapsulation but restore the old
ordering: when there is more than one source, scan the packfiles of
every source first, then consult each source's loose store. Single-source
repositories are unaffected.
stat()scache_tree_fully_valid()Test
t5615adds a regression test: an object stored as a packed delta inan alternate and loose in the main object store.
%(deltabase)provesthe read resolves to the alternate's packfile (nonzero base) rather than
the loose copy (zero oid). It fails without this change and passes with
it.
t5613,t1006, andt0410remain green.Notes
This is a targeted mitigation for the microsoft/git fork so the
regression can be addressed quickly. The change keeps the per-source
encapsulation intact and only alters the search order when alternates are
present. A broader upstream discussion of the ordering may be worthwhile
separately.