Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions odb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions odb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion odb/source-files.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
43 changes: 43 additions & 0 deletions t/t5615-alternate-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)" <in >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 <b-content >/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)" <in >actual &&
test_cmp expect actual
'

test_done
Loading