odb/source-files: fix performance when reading objects with alternates

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]: <CAFXJcxvpKHoVDwE5mBOd=w-A5vPdUmehqr8SHLUD7qv1qB00rA@mail.gmail.com>

Reported-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
Helped-by: Tyrie Vella <tyrielv@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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)

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)" <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_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)" <in >actual &&
+	test_cmp expect actual
+'
+
 test_done