From a5a82a0a20ebd338f439f65c2e61507dee0821ae Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 15 Oct 2026 07:27:55 +0200 Subject: [PATCH] odb/source-files: fix performance when reading objects with alternates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the move towards pluggable object databases we reorganized how we access alternates. Before the introduction of pluggability, we were reading objects by first scanning through packfiles from all alternates before scanning through all loose object directories. With the change though, we are now iterating through the sources one by one, checking each of them for whether they contain the object either as a packed or loose object. Checking whether an object exists in packfiles is comparatively cheap because the list of packs is cached, and the pack indices are likely to be in the page cache. In contrast to that, checking whether an object exists in loose format always incurs one stat(3p) syscall. So scanning for objects one alternate at a time led to a performance regression in in the case where most of the objects are contained in a packfile of an alternate, as reported in [1]. We could fix that issue by introducing another flag that tells the backends to only search the optimized storage backend. But that is adding more complexity to the object database layers that is ultimately only really required because of the way the "files" backend stores its data. So for an alternate backend, such new infrastructure would not really be a good fit. But since then we have moved handling of alternates into the "files" backend itself. One of the consequences of this is that this backend is now having full control over all of its sub-sources, and that also makes it way easier for us to optimize for this access pattern again. Do so by first trying to read objects from packfiles located in any of its object directories before trying to read loose objects. The following benchmark demonstrates that this significantly improves performance in some cases. The benchmark uses the Git repository itself and sets up its object directory as an alternate, while the primary object directory is simply an empty object directory. It then reads all commits enumerated via `git rev-list HEAD` with git-cat-file(1): Benchmark 1: git cat-file --batch-check --unordered (rev = HEAD~) Time (mean ± σ): 179.4 ms ± 2.6 ms [User: 114.6 ms, System: 61.4 ms] Range (min … max): 177.1 ms … 187.7 ms 16 runs Benchmark 2: git cat-file --batch-check --unordered (rev = HEAD) Time (mean ± σ): 95.0 ms ± 1.5 ms [User: 79.8 ms, System: 15.0 ms] Range (min … max): 93.0 ms … 97.9 ms 30 runs Summary git cat-file --batch-check --unordered (rev = HEAD) ran 1.89 ± 0.04 times faster than git cat-file --batch-check --unordered (rev = HEAD~) [1]: Reported-by: Wolfgang Kritzinger Helped-by: Tyrie Vella Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb/source-files.c | 5 +++-- t/t5615-alternate-env.sh | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/odb/source-files.c b/odb/source-files.c index 455f409db0..9268f5aa9a 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -426,6 +426,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source { struct odb_source_files *files = odb_source_files_downcast(source); enum odb_read_status status = ODB_READ_NOT_FOUND; + enum odb_read_status ret; /* * Reading an object may fail even though the object exists, for @@ -434,15 +435,15 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source * prefer the first such error in case multiple reads have failed. */ for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) { - enum odb_read_status ret; - ret = odb_source_read_object_info(&dir->packed->base, oid, oi, flags, status == ODB_READ_NOT_FOUND ? errmsg : NULL); if (!ret) return 0; if (ret != ODB_READ_NOT_FOUND && status == ODB_READ_NOT_FOUND) status = ret; + } + for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) { ret = odb_source_read_object_info(&dir->loose->base, oid, oi, flags, status == ODB_READ_NOT_FOUND ? errmsg : NULL); if (!ret) diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh index 9d6aa2187f..3259852a0f 100755 --- a/t/t5615-alternate-env.sh +++ b/t/t5615-alternate-env.sh @@ -88,4 +88,47 @@ test_expect_success !MINGW 'broken quoting falls back to interpreting raw' ' EOF ' +test_expect_success 'packs across sources are checked before loose objects' ' + # Regression test for a performance issue in which an object that + # resides in an alternate as a packed object caused a spurious loose + # object lookup (a filesystem stat) on the main object store before the + # alternate packfile was consulted. Reading such an object must resolve + # to the alternate packfile, never to a loose copy in the main store. + # + # Build an alternate whose object "B" is stored as a delta in a + # packfile. git deltifies successive versions of a tracked file, so the + # older, shorter blob "B" becomes a delta against the newer, longer + # blob "O". A loose object has no delta base, so %(deltabase) tells us + # which store answered the read: the alternate pack (O) or a loose copy + # (the zero oid). + git init alt-src && + test_seq 1 200 >alt-src/file && + git -C alt-src add file && + git -C alt-src commit -q -m base && + B=$(git -C alt-src rev-parse HEAD:file) && + git -C alt-src cat-file blob "$B" >b-content && + test_seq 1 210 >alt-src/file && + git -C alt-src add file && + git -C alt-src commit -q -m more && + O=$(git -C alt-src rev-parse HEAD:file) && + git -C alt-src repack -adf --window=10 --depth=50 && + + # Precondition: in the alternate, B is a delta based on O. + echo "$B" >in && + echo "$O" >expect && + git -C alt-src cat-file --batch-check="%(deltabase)" actual && + test_cmp expect actual && + + # Main repo: write B as a loose object, before any alternate is active. + git init main && + git -C main hash-object -w --stdin /dev/null && + test_path_is_file "main/.git/objects/$(test_oid_to_path "$B")" && + + # With the alternate active, B must resolve to the alternate packfile + # (deltabase O), not to the main store loose copy (deltabase zero oid). + GIT_ALTERNATE_OBJECT_DIRECTORIES="$PWD/alt-src/.git/objects" \ + git -C main cat-file --batch-check="%(deltabase)" actual && + test_cmp expect actual +' + test_done