diff --git a/odb.c b/odb.c index b94f2077272513..7ed8acec7eab3c 100644 --- a/odb.c +++ b/odb.c @@ -756,6 +756,19 @@ static int register_all_submodule_sources(struct object_database *odb) return ret; } +static int read_object_info_from_sources(struct object_database *odb, + const struct object_id *oid, + struct object_info *oi, + enum object_info_flags flags) +{ + struct odb_source *source; + + for (source = odb->sources; source; source = source->next) + if (!odb_source_read_object_info(source, oid, oi, flags)) + return 0; + return -1; +} + static int do_oid_object_info_extended(struct object_database *odb, const struct object_id *oid, struct object_info *oi, unsigned flags) @@ -781,9 +794,28 @@ static int do_oid_object_info_extended(struct object_database *odb, extern int core_use_gvfs_helper; struct odb_source *source; - for (source = odb->sources; source; source = source->next) - if (!odb_source_read_object_info(source, real, oi, flags)) + /* + * With one or more alternates, scan the packfiles of + * every source before consulting any source's loose + * object store. Otherwise a primary-source loose lookup + * -- a filesystem stat that, without OBJECT_INFO_QUICK, + * bypasses the cached loose index -- runs for every + * object that resides in an alternate's packfile. + * cache_tree_fully_valid() checks many tree objects that + * live in an alternate, so this avoids a stat() per + * object. + */ + if (odb->sources && odb->sources->next) { + if (!read_object_info_from_sources(odb, real, oi, + flags | OBJECT_INFO_SKIP_LOOSE)) return 0; + if (!read_object_info_from_sources(odb, real, oi, + flags | OBJECT_INFO_SKIP_PACKED)) + return 0; + } else if (!read_object_info_from_sources(odb, real, oi, + flags)) { + return 0; + } if (core_use_gvfs_helper && !tried_gvfs_helper) { enum gh_client__created ghc; diff --git a/odb.h b/odb.h index d84e4ccc626af2..64cc3f48b321a6 100644 --- a/odb.h +++ b/odb.h @@ -332,6 +332,18 @@ enum object_info_flags { */ OBJECT_INFO_SECOND_READ = (1 << 4), + /* + * Only consult the packed object store of a source, skipping its loose + * object store (OBJECT_INFO_SKIP_LOOSE), or vice versa + * (OBJECT_INFO_SKIP_PACKED). These are used by + * odb_read_object_info_extended() to scan the packfiles of all sources + * before consulting any source's loose object store, so that an object + * that resides in an alternate's packfile is not preceded by a spurious + * loose-object lookup on an earlier source. + */ + OBJECT_INFO_SKIP_LOOSE = (1 << 5), + OBJECT_INFO_SKIP_PACKED = (1 << 6), + /* * This is meant for bulk prefetching of missing blobs in a partial * clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK. diff --git a/odb/source-files.c b/odb/source-files.c index 1b90ccf346008b..6d6a3864812534 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -55,7 +55,11 @@ static int odb_source_files_read_object_info(struct odb_source *source, { struct odb_source_files *files = odb_source_files_downcast(source); - if (!packfile_store_read_object_info(files->packed, oid, oi, flags) || + if (!(flags & OBJECT_INFO_SKIP_PACKED) && + !packfile_store_read_object_info(files->packed, oid, oi, flags)) + return 0; + + if (!(flags & OBJECT_INFO_SKIP_LOOSE) && !odb_source_read_object_info(&files->loose->base, oid, oi, flags)) return 0; diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh index 1bfeccdeb49958..d82dba65d94b14 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 -e "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