[PATCH 0/4] odb/files: reload alternates on retry again

[PATCH 0/4] odb/files: reload alternates on retry again

[Patrick Steinhardt]

Hi,

we recently noticed that we don't reload alternates anymore when an
object cannot be found. This used to be the case, but broke with
5946a564cd (odb/source: make `read_object_info()` function pluggable,
2026-03-05). And while it's a rather uncommon scenario that alternates
are getting rewritten while a Git process runs we managed to hit some
such scenarios in production systems.

In any case, this patch series fixes that. In practice, only the second
commit is really needed to restore functionality. I was a bit unhappy
about the performance impact of this change though, as fixing the logic
regressed us from 180ms when looking up 20k missing objects to 309ms. So
the last two patches do some optimizations that bring us down to 197ms.

The series is built on top of 47fdf2208c (The 7th batch, 2026-10-19)
with ps/odb-files-alternates-cleanups at f211a3f0b5 (odb/source: drop
`local` flag, 2026-10-15) merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (4):
      odb/source-files: rename alternates-related functions
      odb/source-files: reload alternates on retry
      odb/source-files: avoid rechecking already-known alternates
      odb/source-files: only reload alternates when the file has changed

 odb/source-files.c  | 84 +++++++++++++++++++++++++++++++----------------------
 odb/source-files.h  |  4 +++
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++
 3 files changed, 86 insertions(+), 34 deletions(-)


---
base-commit: c8e565497cff47f6b23a6b96bef6fa82afa11667
change-id: 20261014-pks-odb-files-reload-alternates-dfc6dc010122

[PATCH 1/4] odb/source-files: rename alternates-related functions

[Patrick Steinhardt]

The functions that prepare and add alternates in the "files" backend
were originally part of the object database layer, which explains why
they still have an "odb_" prefix. That prefix does not make much sense
anymore now that those are part of the "files" backend.

Drop the prefix.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 5b4277fbfd..d31da6e5d9 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -348,9 +348,9 @@ static int read_alternates(const char *object_dir, struct strvec *out)
 	return 0;
 }
 
-static void odb_add_alternate_recursively(struct odb_source_files *files,
-					  const char *path,
-					  int depth)
+static void add_alternate_recursively(struct odb_source_files *files,
+				      const char *path,
+				      int depth)
 {
 	struct odb_files_dir *alternate;
 	struct strvec alternates = STRVEC_INIT;
@@ -377,15 +377,15 @@ static void odb_add_alternate_recursively(struct odb_source_files *files,
 		      path);
 	} else {
 		for (size_t i = 0; i < alternates.nr; i++)
-			odb_add_alternate_recursively(files, alternates.v[i], depth + 1);
+			add_alternate_recursively(files, alternates.v[i], depth + 1);
 	}
 
  out:
 	strvec_clear(&alternates);
 }
 
-static void odb_prepare_alternates(struct odb_source_files *files,
-				   const char *alternate_db)
+static void prepare_alternates(struct odb_source_files *files,
+			       const char *alternate_db)
 {
 	struct strvec alternates = STRVEC_INIT;
 
@@ -393,7 +393,7 @@ static void odb_prepare_alternates(struct odb_source_files *files,
 	read_alternates(files->dirs->abspath, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(files, alternates.v[i], 0);
+		add_alternate_recursively(files, alternates.v[i], 0);
 
 	strvec_clear(&alternates);
 }
@@ -410,7 +410,7 @@ static void odb_source_files_prepare(struct odb_source *source,
 	 * for the lifetime of the process.
 	 */
 	if (flags & ODB_PREPARE_FLUSH_CACHES)
-		odb_prepare_alternates(files, NULL);
+		prepare_alternates(files, NULL);
 
 	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
 		odb_source_prepare(&dir->loose->base, flags);
@@ -1299,7 +1299,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	if (!is_absolute_path(object_dir))
 		chdir_notify_register(odb_source_files_reparent, files);
 
-	odb_prepare_alternates(files, alternates);
+	prepare_alternates(files, alternates);
 
 	free(object_dir);
 	free(alternates);

-- 
2.56.0.269.g68944adf51.dirty

[PATCH 2/4] odb/source-files: reload alternates on retry

[Patrick Steinhardt]

When an object ID cannot be found we retry reading it from our packfiles
after we have flushed the respective caches of the packed backends. But
this misses one scenario: it may happen that the object databases'
alternates get rewritten concurrently and all of its objects are moved
to such an alternate. If so, we won't be able to find any of the objects
anymore. This used to be working just fine, but with 5946a564cd
(odb/source: make `read_object_info()` function pluggable, 2026-03-05)
we have accidentally dropped this logic.

This scenario sounds somewhat unlikely, but we've actually been hitting
it in the wild: when running long-lived git-cat-file(1) processes that
we interactively query over many minutes, and that repository gets
forked concurrently, then we move all objects into an object pool, write
a new alternates file, and the git-cat-file(1) process will remain
broken for the rest of its lifetime.

Fix this bug by re-preparing alternates when the object in question
could not be found and then retrying the read. Note that this also
requires us to re-read loose objects, which is an operation that we
previously skipped on the retry. But we only need to do this for the
loose object directories that are actually new.

Note that the result is still incomplete though, as we only reload the
alternates of the local object directory. But alternates are added
recursively, so in theory we'd also have to check whether alternates of
any of the alternates have changed. But we never did that before the
regression, either, so for now we don't care about this edge case.

Furthermore, this change causes a performance regression when asking for
lots of missing objects. But that regression is basically in line with
the performance we had before this mechanism got broken. The following
benchmark demonstrates this when asking for 20k missing commits in an
empty repository with a single empty alternate:

  Benchmark 1: git cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     282.8 ms ±   2.9 ms    [User: 99.8 ms, System: 178.6 ms]
    Range (min … max):   279.7 ms … 288.3 ms    10 runs

  Benchmark 2: git cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     180.0 ms ±   1.7 ms    [User: 55.9 ms, System: 120.3 ms]
    Range (min … max):   178.2 ms … 183.9 ms    16 runs

  Benchmark 3: git cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     187.7 ms ±   3.7 ms    [User: 59.7 ms, System: 124.3 ms]
    Range (min … max):   182.2 ms … 193.7 ms    15 runs

  Benchmark 4: git cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     309.8 ms ±   5.3 ms    [User: 105.9 ms, System: 199.1 ms]
    Range (min … max):   302.5 ms … 316.0 ms    10 runs

  Summary
    git cat-file --batch-check (rev = 5946a564cd) ran
      1.04 ± 0.02 times faster than git cat-file --batch-check (rev = HEAD~)
      1.57 ± 0.02 times faster than git cat-file --batch-check (rev = 5946a564cd~)
      1.72 ± 0.03 times faster than git cat-file --batch-check (rev = HEAD)

There's no helping this though, with the regression fixed we will be
slower compared to before as we have to do more work. But the next
commits will improve the status quo a bit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c  | 12 ++++++++----
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d31da6e5d9..86f3fd1136 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 							      struct strbuf *errmsg)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct odb_files_dir **tail = NULL;
 	enum odb_read_status status = ODB_READ_NOT_FOUND;
 	enum odb_read_status ret;
 	bool retry = false;
@@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 			status = ret;
 	}
 
-	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-		if (retry)
-			break;
-
+	/*
+	 * When we have reprepared alternates then we only re-scan directories
+	 * that were newly added.
+	 */
+	for (struct odb_files_dir *dir = tail ? *tail : 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)
@@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 	}
 
 	if (!retry && !(flags & OBJECT_INFO_QUICK)) {
+		tail = files->dirs_tail;
+		prepare_alternates(files, NULL);
 		retry = true;
 		goto retry;
 	}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 72fc8acc61..f2b24dd278 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1481,4 +1481,36 @@ test_objects_filter "object:type=commit"
 test_objects_filter "object:type=tag"
 test_objects_filter "object:type=tree"
 
+test_expect_success PIPE 'concurrently written alternates are loaded' '
+	test_when_finished "rm -rf repo alternate stdin stdout" &&
+	git init repo &&
+	git init alternate &&
+	oid=$(git -C alternate hash-object -w --stdin </dev/null) &&
+	mkfifo stdin &&
+	mkfifo stdout &&
+	{ git -C repo cat-file --batch-check <stdin >stdout & } &&
+	(
+		exec 8>stdin &&
+		exec 9<stdout &&
+
+		# Read the object the first time, without having alternates
+		# wired up yet. The object should be reported as missing.
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid missing" >expect &&
+		test_cmp expect actual &&
+
+		# Read the object the second time, this time with the alternate
+		# set up. The backend should reload alternates and thus find
+		# the object.
+		echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates &&
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid blob 0" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.56.0.269.g68944adf51.dirty

[PATCH 3/4] odb/source-files: avoid rechecking already-known alternates

[Patrick Steinhardt]

When adding alternates we first check whether the alternate is usable
via `odb_files_dir_is_usable()`. This function performs three checks:

  - It verifies that the alternate is not the same as the primary object
    directory.

  - It verifies that the alternate exists on disk.

  - It verifies that the alternate isn't already known to us by looking
    it up in `dirs_by_path`.

The first two checks hit the filesystem, whereas the last one is a mere
hashmap lookup. But the way the code is currently arranged we perform
the hashmap lookup last, which may cause us to do a couple of syscalls
only to then notice that we already know the alternate anyway, which is
of course quite inefficient.

Furthermore, the first check has gotten more expensive over time. While
we traditionally stored the relative path to the primary object
directory, we nowadays store an absolute path. This change is overall a
sensible one as it brings it in line with alternate object directories,
for which we always used to store absolute paths. But it also results in
more calls to lstat(3p) when calling `strbuf_realpath()` to resolve the
path of the primary object directory, as that syscall is made once per
path component.

Reorder the logic so that we perform the hash lookup first for an easy
win. Using the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     282.5 ms ±   2.9 ms    [User: 100.0 ms, System: 178.4 ms]
    Range (min … max):   279.8 ms … 289.1 ms    10 runs

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     181.1 ms ±   1.7 ms    [User: 53.9 ms, System: 123.4 ms]
    Range (min … max):   178.2 ms … 183.6 ms    16 runs

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     309.7 ms ±   2.7 ms    [User: 108.0 ms, System: 197.4 ms]
    Range (min … max):   305.4 ms … 315.1 ms    10 runs

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     263.4 ms ±   3.6 ms    [User: 92.2 ms, System: 167.1 ms]
    Range (min … max):   254.8 ms … 268.2 ms    11 runs

  Summary
    ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) ran
      1.45 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
      1.56 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
      1.71 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)

So we're faster than the original, non-broken state now, but still
significantly slower compared to the regressed commit where we didn't
reload alternates at all.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 86f3fd1136..917f5e9d25 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -228,16 +228,6 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct hashmap_entry key;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
-
-	/* Detect cases where alternate disappeared */
-	if (!is_directory(path)) {
-		error(_("object directory %s does not exist; "
-			"check .git/objects/info/alternates"),
-		      path);
-		goto out;
-	}
-
 	/*
 	 * Prevent the common mistake of listing the same
 	 * thing twice, or object directory itself.
@@ -249,12 +239,21 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 		hashmap_add(&files->dirs_by_path, &files->dirs->by_path_entry);
 	}
 
+	hashmap_entry_init(&key, strihash(path));
+	if (hashmap_get(&files->dirs_by_path, &key, path))
+		goto out;
+
+	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
 	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
 		goto out;
 
-	hashmap_entry_init(&key, strihash(path));
-	if (hashmap_get(&files->dirs_by_path, &key, path))
+	/* Detect cases where alternate disappeared */
+	if (!is_directory(path)) {
+		error(_("object directory %s does not exist; "
+			"check .git/objects/info/alternates"),
+		      path);
 		goto out;
+	}
 
 	usable = true;
 

-- 
2.56.0.269.g68944adf51.dirty

[PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

In the preceding commit we have fixed a regression where we didn't
re-read the alternates file anymore when we hit a missing object. But
while correct, this change significantly hampers performance, even
though we're roughly at the same level compared to before the
regression.

But the checks are unnecessarily expensive, as we always read and parse
the alternates file even in the case where it has not changed. This of
course translates into a lot of busywork for the common case, where the
"objects/info/alternates" is still the exact same.

Improve this by using a `struct stat_validity`. Like this we can easily
short-circuit reading the alternates file in case we can determine via a
stat(3p) call that the file hasn't changed. This significantly improves
performance:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    Time (mean ± σ):     280.7 ms ±   2.1 ms    [User: 95.7 ms, System: 181.0 ms]
    Range (min … max):   277.9 ms … 285.0 ms    10 runs

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd)
    Time (mean ± σ):     179.5 ms ±   1.7 ms    [User: 60.5 ms, System: 115.6 ms]
    Range (min … max):   177.6 ms … 183.2 ms    16 runs

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    Time (mean ± σ):     257.2 ms ±   3.5 ms    [User: 94.8 ms, System: 158.3 ms]
    Range (min … max):   252.6 ms … 262.1 ms    11 runs

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    Time (mean ± σ):     197.4 ms ±   3.8 ms    [User: 66.9 ms, System: 126.7 ms]
    Range (min … max):   192.3 ms … 203.3 ms    14 runs

  Summary
    ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) ran
      1.10 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
      1.43 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
      1.56 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)

We're significantly faster compared to the original state now and are
almost as fast as the broken state where we don't reload alternates at
all. This is probably good enough for now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 31 ++++++++++++++++++++++---------
 odb/source-files.h |  4 ++++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 917f5e9d25..8ac4f7f380 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -73,6 +73,7 @@ static void odb_source_files_free(struct odb_source *source)
 		files->dirs = next;
 	}
 	hashmap_clear(&files->dirs_by_path);
+	stat_validity_clear(&files->alternates_validity);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -329,22 +330,33 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static int read_alternates(const char *object_dir, struct strvec *out)
+static void read_alternates(const char *object_dir, struct strvec *out,
+			    struct stat_validity *validity)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
 	path = xstrfmt("%s/info/alternates", object_dir);
-	if (strbuf_read_file(&buf, path, 1024) < 0) {
-		warn_on_fopen_errors(path);
-		free(path);
-		return 0;
+	if (!validity || !stat_validity_check(validity, path)) {
+		int fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			warn_on_fopen_errors(path);
+			if (validity)
+				stat_validity_clear(validity);
+		} else {
+			if (validity)
+				stat_validity_update(validity, fd);
+			if (strbuf_read(&buf, fd, 1024) < 0)
+				warning_errno(_("unable to read alternates file '%s'"),
+					      path);
+			else
+				parse_alternates(buf.buf, '\n', object_dir, out);
+			close(fd);
+		}
 	}
-	parse_alternates(buf.buf, '\n', object_dir, out);
 
 	strbuf_release(&buf);
 	free(path);
-	return 0;
 }
 
 static void add_alternate_recursively(struct odb_source_files *files,
@@ -370,7 +382,7 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	hashmap_add(&files->dirs_by_path, &alternate->by_path_entry);
 
 	/* recursively add alternates */
-	read_alternates(alternate->abspath, &alternates);
+	read_alternates(alternate->abspath, &alternates, NULL);
 	if (alternates.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      path);
@@ -389,7 +401,8 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->dirs->abspath, &alternates);
+	read_alternates(files->dirs->abspath, &alternates,
+			&files->alternates_validity);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);
diff --git a/odb/source-files.h b/odb/source-files.h
index 837e890304..b0795cd384 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "statinfo.h"
 
 struct odb_source_loose;
 struct odb_source_packed;
@@ -66,6 +67,9 @@ struct odb_source_files {
 	 * determined by "core.ignoreCase".
 	 */
 	int dirs_paths_icase;
+
+	/* Stat data of the "info/alternates" file. */
+	struct stat_validity alternates_validity;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.269.g68944adf51.dirty

Re: [PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> +	if (!validity || !stat_validity_check(validity, path)) {
> +		int fd = open(path, O_RDONLY);
> +		if (fd < 0) {
> +			warn_on_fopen_errors(path);
> +			if (validity)
> +				stat_validity_clear(validity);
> +		} else {
> +			if (validity)
> +				stat_validity_update(validity, fd);
> +			if (strbuf_read(&buf, fd, 1024) < 0)
> +				warning_errno(_("unable to read alternates file '%s'"),
> +					      path);
> +			else
> +				parse_alternates(buf.buf, '\n', object_dir, out);
> +			close(fd);

When we update validity and then fail to strbuf_read(), say due to a
transient I/O error or something that was caused by somebody else
mucking with our alternates list at the same time, we warn, but do
not roll back the validity update even though we failed to parse.  I
wonder if it is safer to run the validity update only after
successfully read the alternates file, and if the extra safety is
worth it?

Re: [PATCH 4/4] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

On Tue, Oct 20, 2026 at 12:59:58PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > +	if (!validity || !stat_validity_check(validity, path)) {
> > +		int fd = open(path, O_RDONLY);
> > +		if (fd < 0) {
> > +			warn_on_fopen_errors(path);
> > +			if (validity)
> > +				stat_validity_clear(validity);
> > +		} else {
> > +			if (validity)
> > +				stat_validity_update(validity, fd);
> > +			if (strbuf_read(&buf, fd, 1024) < 0)
> > +				warning_errno(_("unable to read alternates file '%s'"),
> > +					      path);
> > +			else
> > +				parse_alternates(buf.buf, '\n', object_dir, out);
> > +			close(fd);
> 
> When we update validity and then fail to strbuf_read(), say due to a
> transient I/O error or something that was caused by somebody else
> mucking with our alternates list at the same time, we warn, but do
> not roll back the validity update even though we failed to parse.  I
> wonder if it is safer to run the validity update only after
> successfully read the alternates file, and if the extra safety is
> worth it?

Good question. I doubt that the extra safety would really be worth it in
the general case as it's rather unlikely that we can open the path but
then not read from it. But that doesn't feel like a good-enough argument
to be sloppy, because "unlikely" doesn't mean "impossible".

So we could do something like this instead:

	if (!validity || !stat_validity_check(validity, path)) {
		bool successful = false;
		int fd;

		fd = open(path, O_RDONLY);
		if (fd < 0) {
			warn_on_fopen_errors(path);
		} else if (strbuf_read(&buf, fd, 1024) < 0) {
			warning_errno(_("unable to read alternates file '%s'"),
				      path);
		} else {
			parse_alternates(buf.buf, '\n', object_dir, out);
			successful = true;
		}

		if (validity) {
			if (successful)
				stat_validity_update(validity, fd);
			else
				stat_validity_clear(validity);
		}

		if (fd >= 0)
			close(fd);
	}

I've queued that change locally for now.

Thanks!

Patrick

Re: [PATCH 2/4] odb/source-files: reload alternates on retry

[Christian Couder]

On Tue, Oct 20, 2026 at 11:01 AM Patrick Steinhardt <ps@pks.im> wrote:

> diff --git a/odb/source-files.c b/odb/source-files.c
> index d31da6e5d9..86f3fd1136 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>                                                               struct strbuf *errmsg)
>  {
>         struct odb_source_files *files = odb_source_files_downcast(source);
> +       struct odb_files_dir **tail = NULL;
>         enum odb_read_status status = ODB_READ_NOT_FOUND;
>         enum odb_read_status ret;
>         bool retry = false;
> @@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>                         status = ret;
>         }
>
> -       for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
> -               if (retry)
> -                       break;
> -
> +       /*
> +        * When we have reprepared alternates then we only re-scan directories
> +        * that were newly added.
> +        */
> +       for (struct odb_files_dir *dir = tail ? *tail : 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)
> @@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
>         }
>
>         if (!retry && !(flags & OBJECT_INFO_QUICK)) {
> +               tail = files->dirs_tail;
> +               prepare_alternates(files, NULL);

This calls `read_alternates(files->dirs->abspath, ...)`, but when we
use a temporary primary directory, `files->dirs` is that temporary
primary directory. The prepare_alternates() call will then quietly
find nothing new because temporary primary directories have no
alternates.

Before 5946a564cd (odb/source: make `read_object_info()` function
pluggable, 2026-03-05), when there was a temporary primary directory,
reloading the alternates also found nothing new for the same reason
(no alternates).

So we could think that there is no behavior change, but in the
meantime there was bdee7b3013 (builtin/receive-pack: stage incoming
objects via ODB transactions, 2026-07-10) which made the quarantine
the temporary primary directory in receive-pack. Before that commit,
receive-pack only passed its quarantine to child processes through
environment variables, so its own lookups reloaded the repository's
real alternates.

I wonder if it would be better to always reload the repository's real
alternates, instead of `files->dirs`'s alternates. This way, objects
that moved to an alternate would still be found, not only during a
receive-pack like before 5946a564cd, but whenever a temporary primary
directory is used.

It looks like with your 4/4 patch this improvement would add no extra cost.

Then `files->dirs` should also likely be replaced by the repository's
own object directory in the following places inside
odb_files_dir_is_usable():

- in the hashmap_entry_init() call that seeds the duplicate map, and
- in strbuf_realpath() when we are supposed to compare against the
resolved path of the primary.

These are not reachable today when a temporary primary directory is
installed, as no alternates are read then. But they would become
reachable if we reload the repository's real alternates, instead of
`files->dirs`'s alternates.

>                 retry = true;
>                 goto retry;
>         }

Re: [PATCH 2/4] odb/source-files: reload alternates on retry

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 04:32:34PM +0200, Christian Couder wrote:
> On Tue, Oct 20, 2026 at 11:01 AM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > diff --git a/odb/source-files.c b/odb/source-files.c
> > index d31da6e5d9..86f3fd1136 100644
> > --- a/odb/source-files.c
> > +++ b/odb/source-files.c
> > @@ -425,6 +425,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >                                                               struct strbuf *errmsg)
> >  {
> >         struct odb_source_files *files = odb_source_files_downcast(source);
> > +       struct odb_files_dir **tail = NULL;
> >         enum odb_read_status status = ODB_READ_NOT_FOUND;
> >         enum odb_read_status ret;
> >         bool retry = false;
> > @@ -448,10 +449,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >                         status = ret;
> >         }
> >
> > -       for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
> > -               if (retry)
> > -                       break;
> > -
> > +       /*
> > +        * When we have reprepared alternates then we only re-scan directories
> > +        * that were newly added.
> > +        */
> > +       for (struct odb_files_dir *dir = tail ? *tail : 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)
> > @@ -461,6 +463,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
> >         }
> >
> >         if (!retry && !(flags & OBJECT_INFO_QUICK)) {
> > +               tail = files->dirs_tail;
> > +               prepare_alternates(files, NULL);
> 
> This calls `read_alternates(files->dirs->abspath, ...)`, but when we
> use a temporary primary directory, `files->dirs` is that temporary
> primary directory. The prepare_alternates() call will then quietly
> find nothing new because temporary primary directories have no
> alternates.

*sigh* True. I really wish that this in-place modification of dirs just
went away entirely. But that's a bigger change to make.

[snip]
> I wonder if it would be better to always reload the repository's real
> alternates, instead of `files->dirs`'s alternates. This way, objects
> that moved to an alternate would still be found, not only during a
> receive-pack like before 5946a564cd, but whenever a temporary primary
> directory is used.

Yeah, we should probably use `files->base.path` instead.

Thanks!

Patrick

[PATCH v2 0/6] odb/files: reload alternates on retry again

[Patrick Steinhardt]

Hi,

we recently noticed that we don't reload alternates anymore when an
object cannot be found. This used to be the case, but broke with
5946a564cd (odb/source: make `read_object_info()` function pluggable,
2026-03-05). And while it's a rather uncommon scenario that alternates
are getting rewritten while a Git process runs we managed to hit some
such scenarios in production systems.

In any case, this patch series fixes that. In practice, only the second
commit is really needed to restore functionality. I was a bit unhappy
about the performance impact of this change though, as fixing the logic
regressed us from 180ms when looking up 20k missing objects to 309ms. So
the last two patches do some optimizations that bring us down to 197ms.

The series is built on top of 47fdf2208c (The 7th batch, 2026-10-19)
with ps/odb-files-alternates-cleanups at f211a3f0b5 (odb/source: drop
`local` flag, 2026-10-15) merged into it.

Changes in v2:
  - We now properly load alternates from the primary object directory
    again even in the case where a quarantine directory is active, based
    on Christian's feedback.
  - I redid how we deduplicate alternates as a result of that. The
    resulting logic is simpler now while still giving the same
    guarantees as before.
  - Link to v1: https://patch.msgid.link/20261020-pks-odb-files-reload-alternates-v1-0-d9e156323836@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (6):
      odb/source-files: rename alternates-related functions
      odb/source-files: always load alternates via primary object directory
      odb/source-files: reload alternates on retry
      odb/source-files: simplify directory duplication detection
      odb/source-files: avoid stat'ing already-known alternates
      odb/source-files: only reload alternates when the file has changed

 odb/source-files.c  | 187 ++++++++++++++++++++++++++--------------------------
 odb/source-files.h  |  21 +++---
 t/t1006-cat-file.sh |  32 +++++++++
 3 files changed, 134 insertions(+), 106 deletions(-)

Range-diff versus v1:

1:  94a0d4298a = 1:  1094828c5a odb/source-files: rename alternates-related functions
-:  ---------- > 2:  909f28fda5 odb/source-files: always load alternates via primary object directory
2:  73e7fa850f ! 3:  b5de5bf034 odb/source-files: reload alternates on retry
    @@ Commit message
         broken for the rest of its lifetime.
     
         Fix this bug by re-preparing alternates when the object in question
    -    could not be found and then retrying the read. Note that this also
    -    requires us to re-read loose objects, which is an operation that we
    -    previously skipped on the retry. But we only need to do this for the
    -    loose object directories that are actually new.
    +    could not be found and then retrying the read. A couple things to note:
     
    -    Note that the result is still incomplete though, as we only reload the
    -    alternates of the local object directory. But alternates are added
    -    recursively, so in theory we'd also have to check whether alternates of
    -    any of the alternates have changed. But we never did that before the
    -    regression, either, so for now we don't care about this edge case.
    +      - This requires us to re-read loose objects, which is an operation
    +        that we previously skipped on the retry. But we only need to do this
    +        for the loose object directories that are actually new.
    +
    +      - The result is still incomplete, as we only reload the alternates of
    +        the primary object directory. But alternates are added recursively,
    +        so in theory we'd also have to check whether alternates of any of
    +        the alternates have changed. But we never did that before the
    +        regression, either, so for now we don't care about this edge case.
    +
    +      - It may happen that we reload when a quarantine directory is active.
    +        If no alternates existed before that reload we may not have lazily
    +        populated the map of object directories yet. So the assertion in
    +        `odb_files_dir_is_usable()` does not necessarily hold anymore, and
    +        we now have to populate the map with all object directories.
     
         Furthermore, this change causes a performance regression when asking for
         lots of missing objects. But that regression is basically in line with
    @@ Commit message
         benchmark demonstrates this when asking for 20k missing commits in an
         empty repository with a single empty alternate:
     
    -      Benchmark 1: git cat-file --batch-check (rev = 5946a564cd~)
    -        Time (mean ± σ):     282.8 ms ±   2.9 ms    [User: 99.8 ms, System: 178.6 ms]
    -        Range (min … max):   279.7 ms … 288.3 ms    10 runs
    -
    -      Benchmark 2: git cat-file --batch-check (rev = 5946a564cd)
    -        Time (mean ± σ):     180.0 ms ±   1.7 ms    [User: 55.9 ms, System: 120.3 ms]
    -        Range (min … max):   178.2 ms … 183.9 ms    16 runs
    +      Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    289.9 ms  ±     3.4 ms     285.8 ms  …   307.5 ms
    +        Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB
     
    -      Benchmark 3: git cat-file --batch-check (rev = HEAD~)
    -        Time (mean ± σ):     187.7 ms ±   3.7 ms    [User: 59.7 ms, System: 124.3 ms]
    -        Range (min … max):   182.2 ms … 193.7 ms    15 runs
    +      Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    184.8 ms  ±     2.5 ms     181.5 ms  …   191.9 ms           -36.2%  (1.6x faster)
    +        Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%
     
    -      Benchmark 4: git cat-file --batch-check (rev = HEAD)
    -        Time (mean ± σ):     309.8 ms ±   5.3 ms    [User: 105.9 ms, System: 199.1 ms]
    -        Range (min … max):   302.5 ms … 316.0 ms    10 runs
    +      Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    189.1 ms  ±     2.7 ms     185.9 ms  …   196.1 ms           -34.8%  (1.5x faster)
    +        Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.9%
     
    -      Summary
    -        git cat-file --batch-check (rev = 5946a564cd) ran
    -          1.04 ± 0.02 times faster than git cat-file --batch-check (rev = HEAD~)
    -          1.57 ± 0.02 times faster than git cat-file --batch-check (rev = 5946a564cd~)
    -          1.72 ± 0.03 times faster than git cat-file --batch-check (rev = HEAD)
    +      Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    293.2 ms  ±     2.6 ms     289.3 ms  …   303.2 ms            +1.1%
    +        Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.4%
     
         There's no helping this though, with the regression fixed we will be
         slower compared to before as we have to do more work. But the next
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## odb/source-files.c ##
    +@@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files *files,
    + 	 * thing twice, or object directory itself.
    + 	 */
    + 	if (!hashmap_get_size(&files->dirs_by_path)) {
    +-		assert(!files->dirs->next);
    +-		hashmap_entry_init(&files->dirs->by_path_entry,
    +-				   strihash(files->dirs->abspath));
    +-		hashmap_add(&files->dirs_by_path, &files->dirs->by_path_entry);
    ++		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
    ++			hashmap_entry_init(&dir->by_path_entry,
    ++					   strihash(dir->abspath));
    ++			hashmap_add(&files->dirs_by_path, &dir->by_path_entry);
    ++		}
    + 	}
    + 
    + 	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
     @@ odb/source-files.c: static enum odb_read_status odb_source_files_read_object_info(struct odb_source
      							      struct strbuf *errmsg)
      {
3:  af6c6888db < -:  ---------- odb/source-files: avoid rechecking already-known alternates
-:  ---------- > 4:  4c5a11e534 odb/source-files: simplify directory duplication detection
-:  ---------- > 5:  0d1d468a21 odb/source-files: avoid stat'ing already-known alternates
4:  561c8ea547 ! 6:  fdaeeff929 odb/source-files: only reload alternates when the file has changed
    @@ Metadata
      ## Commit message ##
         odb/source-files: only reload alternates when the file has changed
     
    -    In the preceding commit we have fixed a regression where we didn't
    -    re-read the alternates file anymore when we hit a missing object. But
    -    while correct, this change significantly hampers performance, even
    -    though we're roughly at the same level compared to before the
    -    regression.
    +    In a preceding commit we have fixed a regression where we didn't re-read
    +    the alternates file anymore when we hit a missing object. But while
    +    correct, this change significantly hampers performance, even though
    +    we're roughly at the same level compared to before the regression.
     
         But the checks are unnecessarily expensive, as we always read and parse
         the alternates file even in the case where it has not changed. This of
    @@ Commit message
         stat(3p) call that the file hasn't changed. This significantly improves
         performance:
     
    -      Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    -        Time (mean ± σ):     280.7 ms ±   2.1 ms    [User: 95.7 ms, System: 181.0 ms]
    -        Range (min … max):   277.9 ms … 285.0 ms    10 runs
    +      Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    288.5 ms  ±     2.3 ms     284.9 ms  …   293.7 ms
    +        Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB
     
    -      Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd)
    -        Time (mean ± σ):     179.5 ms ±   1.7 ms    [User: 60.5 ms, System: 115.6 ms]
    -        Range (min … max):   177.6 ms … 183.2 ms    16 runs
    +      Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    185.3 ms  ±     3.4 ms     181.2 ms  …   200.1 ms           -35.8%  (1.6x faster)
    +        Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%
     
    -      Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    -        Time (mean ± σ):     257.2 ms ±   3.5 ms    [User: 94.8 ms, System: 158.3 ms]
    -        Range (min … max):   252.6 ms … 262.1 ms    11 runs
    +      Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    262.7 ms  ±     2.7 ms     257.1 ms  …   268.0 ms            -9.0%
    +        Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.5%
     
    -      Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    -        Time (mean ± σ):     197.4 ms ±   3.8 ms    [User: 66.9 ms, System: 126.7 ms]
    -        Range (min … max):   192.3 ms … 203.3 ms    14 runs
    -
    -      Summary
    -        ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) ran
    -          1.10 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD)
    -          1.43 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~)
    -          1.56 ± 0.02 times faster than ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~)
    +      Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
    +                      mean     ±       σ          min     …     max
    +        Wall Time    199.5 ms  ±     2.5 ms     195.4 ms  …   207.8 ms           -30.9%  (1.4x faster)
    +        Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.0 MiB           +4.3%
     
         We're significantly faster compared to the original state now and are
         almost as fast as the broken state where we don't reload alternates at
    @@ odb/source-files.c
     @@ odb/source-files.c: static void odb_source_files_free(struct odb_source *source)
      		files->dirs = next;
      	}
    - 	hashmap_clear(&files->dirs_by_path);
    + 	strset_clear(&files->dir_paths);
     +	stat_validity_clear(&files->alternates_validity);
      
      	odb_source_release(&files->base);
    @@ odb/source-files.c: static void parse_alternates(const char *string,
     -		free(path);
     -		return 0;
     +	if (!validity || !stat_validity_check(validity, path)) {
    -+		int fd = open(path, O_RDONLY);
    ++		bool successful = false;
    ++		int fd;
    ++
    ++		fd = open(path, O_RDONLY);
     +		if (fd < 0) {
     +			warn_on_fopen_errors(path);
    -+			if (validity)
    -+				stat_validity_clear(validity);
    ++		} else if (strbuf_read(&buf, fd, 1024) < 0) {
    ++			warning_errno(_("unable to read alternates file '%s'"),
    ++				      path);
     +		} else {
    -+			if (validity)
    ++			parse_alternates(buf.buf, '\n', object_dir, out);
    ++			successful = true;
    ++		}
    ++
    ++		if (validity) {
    ++			if (successful)
     +				stat_validity_update(validity, fd);
    -+			if (strbuf_read(&buf, fd, 1024) < 0)
    -+				warning_errno(_("unable to read alternates file '%s'"),
    -+					      path);
     +			else
    -+				parse_alternates(buf.buf, '\n', object_dir, out);
    -+			close(fd);
    ++				stat_validity_clear(validity);
     +		}
    ++
    ++		if (fd >= 0)
    ++			close(fd);
      	}
     -	parse_alternates(buf.buf, '\n', object_dir, out);
      
    @@ odb/source-files.c: static void parse_alternates(const char *string,
      
      static void add_alternate_recursively(struct odb_source_files *files,
     @@ odb/source-files.c: static void add_alternate_recursively(struct odb_source_files *files,
    - 	hashmap_add(&files->dirs_by_path, &alternate->by_path_entry);
    + 	}
      
      	/* recursively add alternates */
     -	read_alternates(alternate->abspath, &alternates);
    @@ odb/source-files.c: static void prepare_alternates(struct odb_source_files *file
      	struct strvec alternates = STRVEC_INIT;
      
      	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
    --	read_alternates(files->dirs->abspath, &alternates);
    -+	read_alternates(files->dirs->abspath, &alternates,
    +-	read_alternates(files->base.path, &alternates);
    ++	read_alternates(files->base.path, &alternates,
     +			&files->alternates_validity);
      
      	for (size_t i = 0; i < alternates.nr; i++)
    @@ odb/source-files.h
      
      #include "odb/source.h"
     +#include "statinfo.h"
    + #include "strmap.h"
      
      struct odb_source_loose;
    - struct odb_source_packed;
     @@ odb/source-files.h: struct odb_source_files {
      	 * determined by "core.ignoreCase".
      	 */
    - 	int dirs_paths_icase;
    + 	int dir_paths_icase;
     +
     +	/* Stat data of the "info/alternates" file. */
     +	struct stat_validity alternates_validity;

---
base-commit: c8e565497cff47f6b23a6b96bef6fa82afa11667
change-id: 20261014-pks-odb-files-reload-alternates-dfc6dc010122

[PATCH v2 1/6] odb/source-files: rename alternates-related functions

[Patrick Steinhardt]

The functions that prepare and add alternates in the "files" backend
were originally part of the object database layer, which explains why
they still have an "odb_" prefix. That prefix does not make much sense
anymore now that those are part of the "files" backend.

Drop the prefix.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 5b4277fbfd..d31da6e5d9 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -348,9 +348,9 @@ static int read_alternates(const char *object_dir, struct strvec *out)
 	return 0;
 }
 
-static void odb_add_alternate_recursively(struct odb_source_files *files,
-					  const char *path,
-					  int depth)
+static void add_alternate_recursively(struct odb_source_files *files,
+				      const char *path,
+				      int depth)
 {
 	struct odb_files_dir *alternate;
 	struct strvec alternates = STRVEC_INIT;
@@ -377,15 +377,15 @@ static void odb_add_alternate_recursively(struct odb_source_files *files,
 		      path);
 	} else {
 		for (size_t i = 0; i < alternates.nr; i++)
-			odb_add_alternate_recursively(files, alternates.v[i], depth + 1);
+			add_alternate_recursively(files, alternates.v[i], depth + 1);
 	}
 
  out:
 	strvec_clear(&alternates);
 }
 
-static void odb_prepare_alternates(struct odb_source_files *files,
-				   const char *alternate_db)
+static void prepare_alternates(struct odb_source_files *files,
+			       const char *alternate_db)
 {
 	struct strvec alternates = STRVEC_INIT;
 
@@ -393,7 +393,7 @@ static void odb_prepare_alternates(struct odb_source_files *files,
 	read_alternates(files->dirs->abspath, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(files, alternates.v[i], 0);
+		add_alternate_recursively(files, alternates.v[i], 0);
 
 	strvec_clear(&alternates);
 }
@@ -410,7 +410,7 @@ static void odb_source_files_prepare(struct odb_source *source,
 	 * for the lifetime of the process.
 	 */
 	if (flags & ODB_PREPARE_FLUSH_CACHES)
-		odb_prepare_alternates(files, NULL);
+		prepare_alternates(files, NULL);
 
 	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
 		odb_source_prepare(&dir->loose->base, flags);
@@ -1299,7 +1299,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	if (!is_absolute_path(object_dir))
 		chdir_notify_register(odb_source_files_reparent, files);
 
-	odb_prepare_alternates(files, alternates);
+	prepare_alternates(files, alternates);
 
 	free(object_dir);
 	free(alternates);

-- 
2.56.0.307.g5fe7bd680c.dirty

[PATCH v2 2/6] odb/source-files: always load alternates via primary object directory

[Patrick Steinhardt]

The files source tracks a list of object directories. The head of the
list is typically expected to be the primary object directory, whereas
the tail is the list of alternates linked to it. This assumption does
not always hold though, as transactions cause us to prepend another
quarantine object directory to the list.

This is not much of a problem now, but in a subsequent commit we're
about to re-read "objects/info/alternates" when we hit a missing
objects. So we may start calling `prepare_alternates()` in situations
where the head of the list is _not_ the primary object directory, but if
so we would now try to read alternates from the quarantine directory and
not from the primary object directory.

Fix this issue by instead using the path of the "files" source both when
reading alternates in `prepare_alternates()` and when deduplicating the
primary object directory in `odb_files_dir_is_usable()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d31da6e5d9..c0375e7cb7 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -228,7 +228,7 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct hashmap_entry key;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
+	strbuf_realpath(&normalized_objdir, files->base.path, 1);
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
@@ -390,7 +390,7 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->dirs->abspath, &alternates);
+	read_alternates(files->base.path, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);

-- 
2.56.0.307.g5fe7bd680c.dirty

[PATCH v2 3/6] odb/source-files: reload alternates on retry

[Patrick Steinhardt]

When an object ID cannot be found we retry reading it from our packfiles
after we have flushed the respective caches of the packed backends. But
this misses one scenario: it may happen that the object databases'
alternates get rewritten concurrently and all of its objects are moved
to such an alternate. If so, we won't be able to find any of the objects
anymore. This used to be working just fine, but with 5946a564cd
(odb/source: make `read_object_info()` function pluggable, 2026-03-05)
we have accidentally dropped this logic.

This scenario sounds somewhat unlikely, but we've actually been hitting
it in the wild: when running long-lived git-cat-file(1) processes that
we interactively query over many minutes, and that repository gets
forked concurrently, then we move all objects into an object pool, write
a new alternates file, and the git-cat-file(1) process will remain
broken for the rest of its lifetime.

Fix this bug by re-preparing alternates when the object in question
could not be found and then retrying the read. A couple things to note:

  - This requires us to re-read loose objects, which is an operation
    that we previously skipped on the retry. But we only need to do this
    for the loose object directories that are actually new.

  - The result is still incomplete, as we only reload the alternates of
    the primary object directory. But alternates are added recursively,
    so in theory we'd also have to check whether alternates of any of
    the alternates have changed. But we never did that before the
    regression, either, so for now we don't care about this edge case.

  - It may happen that we reload when a quarantine directory is active.
    If no alternates existed before that reload we may not have lazily
    populated the map of object directories yet. So the assertion in
    `odb_files_dir_is_usable()` does not necessarily hold anymore, and
    we now have to populate the map with all object directories.

Furthermore, this change causes a performance regression when asking for
lots of missing objects. But that regression is basically in line with
the performance we had before this mechanism got broken. The following
benchmark demonstrates this when asking for 20k missing commits in an
empty repository with a single empty alternate:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.9 ms  ±     3.4 ms     285.8 ms  …   307.5 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    184.8 ms  ±     2.5 ms     181.5 ms  …   191.9 ms           -36.2%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    189.1 ms  ±     2.7 ms     185.9 ms  …   196.1 ms           -34.8%  (1.5x faster)
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.9%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    293.2 ms  ±     2.6 ms     289.3 ms  …   303.2 ms            +1.1%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.4%

There's no helping this though, with the regression fixed we will be
slower compared to before as we have to do more work. But the next
commits will improve the status quo a bit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c  | 21 +++++++++++++--------
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index c0375e7cb7..d66949b7de 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -243,10 +243,11 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	 * thing twice, or object directory itself.
 	 */
 	if (!hashmap_get_size(&files->dirs_by_path)) {
-		assert(!files->dirs->next);
-		hashmap_entry_init(&files->dirs->by_path_entry,
-				   strihash(files->dirs->abspath));
-		hashmap_add(&files->dirs_by_path, &files->dirs->by_path_entry);
+		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
+			hashmap_entry_init(&dir->by_path_entry,
+					   strihash(dir->abspath));
+			hashmap_add(&files->dirs_by_path, &dir->by_path_entry);
+		}
 	}
 
 	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
@@ -425,6 +426,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 							      struct strbuf *errmsg)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct odb_files_dir **tail = NULL;
 	enum odb_read_status status = ODB_READ_NOT_FOUND;
 	enum odb_read_status ret;
 	bool retry = false;
@@ -448,10 +450,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 			status = ret;
 	}
 
-	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-		if (retry)
-			break;
-
+	/*
+	 * When we have reprepared alternates then we only re-scan directories
+	 * that were newly added.
+	 */
+	for (struct odb_files_dir *dir = tail ? *tail : 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)
@@ -461,6 +464,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 	}
 
 	if (!retry && !(flags & OBJECT_INFO_QUICK)) {
+		tail = files->dirs_tail;
+		prepare_alternates(files, NULL);
 		retry = true;
 		goto retry;
 	}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 72fc8acc61..f2b24dd278 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1481,4 +1481,36 @@ test_objects_filter "object:type=commit"
 test_objects_filter "object:type=tag"
 test_objects_filter "object:type=tree"
 
+test_expect_success PIPE 'concurrently written alternates are loaded' '
+	test_when_finished "rm -rf repo alternate stdin stdout" &&
+	git init repo &&
+	git init alternate &&
+	oid=$(git -C alternate hash-object -w --stdin </dev/null) &&
+	mkfifo stdin &&
+	mkfifo stdout &&
+	{ git -C repo cat-file --batch-check <stdin >stdout & } &&
+	(
+		exec 8>stdin &&
+		exec 9<stdout &&
+
+		# Read the object the first time, without having alternates
+		# wired up yet. The object should be reported as missing.
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid missing" >expect &&
+		test_cmp expect actual &&
+
+		# Read the object the second time, this time with the alternate
+		# set up. The backend should reload alternates and thus find
+		# the object.
+		echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates &&
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid blob 0" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.56.0.307.g5fe7bd680c.dirty

[PATCH v2 4/6] odb/source-files: simplify directory duplication detection

[Patrick Steinhardt]

When loading alternates it may happen that we try to add the same
alternate multiple times, either because "info/alternates" contains
duplicate entries or because one of its alternates matches the primary
object directory. We try to detect such duplicates via two mechanisms:

  - We have a hashmap of `struct odb_files_dir`, mapped by their
    absolute paths. If the path of an alternate is already present in
    this map we skip it.

  - We have another, explicit check to check whether the path of an
    alternate matches the primary object directory.

We can simplify both of these mechanisms by turning the map into a
strset, as we never access the values of the map. On the one hand this
gives us simpler interfaces to access the set. And on the other hand
this allows us to add two entries for the primary object directory: one
for the real path, and one for the absolute path.

Note that there is one gotcha though: when "core.ignorecase" is set then
the expectation is that we deduplicate alternates case-insensitively.
But we can also make that work rather easily by downcasing all paths we
add in that case.

Despite being significantly simpler, this also gives us a speedup
because we don't have to resolve the realpath of the primary object
directory on every call to `odb_files_dir_is_usable()` anymore. Using
the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.0 ms  ±     2.2 ms     285.8 ms  …   295.1 ms
    Memory         5.6 MiB ±     0.1 MiB      5.4 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    183.8 ms  ±     1.2 ms     181.5 ms  …   186.9 ms           -36.4%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.3 MiB …     5.7 MiB           -0.5%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    293.2 ms  ±     2.7 ms     288.8 ms  …   299.7 ms            +1.5%
    Memory         5.9 MiB ±     0.1 MiB      5.7 MiB …     6.1 MiB           +5.0%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    266.5 ms  ±     2.8 ms     262.6 ms  …   276.7 ms            -7.8%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +5.1%

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 103 +++++++++++++++++++----------------------------------
 odb/source-files.h |  17 ++++-----
 2 files changed, 43 insertions(+), 77 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d66949b7de..22e96828d2 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -72,7 +72,7 @@ static void odb_source_files_free(struct odb_source *source)
 		odb_files_dir_free(files->dirs);
 		files->dirs = next;
 	}
-	hashmap_clear(&files->dirs_by_path);
+	strset_clear(&files->dir_paths);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -176,60 +176,15 @@ static int odb_source_files_create_on_disk(struct odb_source *source,
 	return ret;
 }
 
-/*
- * NEEDSWORK: we're using "core.ignoreCase" to deduplicate alternates that
- * _may_ be the same. This requires quite a bit of boilerplate for dubious
- * benefit:
- *
- *   - Duplicating alternates should really only lead to regressed performance.
- *
- *   - We don't properly resolve symlinks or mointpoints, so we may still end
- *     up duplicating alternates.
- *
- *   - The value may be lying, in which case we might deduplicate alternates
- *     that are in fact not mapping to the same directory.
- *
- * We should investigate whether we can remove this whole mechanism outright.
- */
-static int odb_files_dir_paths_cmp(struct odb_source_files *files,
-				   const char *a, const char *b)
-{
-	if (files->dirs_paths_icase < 0) {
-		int icase = 0;
-		repo_config_get_bool(files->base.odb->repo, "core.ignorecase", &icase);
-		files->dirs_paths_icase = icase;
-	}
-
-	return files->dirs_paths_icase ? strcasecmp(a, b) : strcmp(a, b);
-}
-
-static int odb_files_dir_by_path_cmp(const void *cb_data,
-				     const struct hashmap_entry *entry,
-				     const struct hashmap_entry *entry_or_key,
-				     const void *keydata)
-{
-	struct odb_source_files *files = (struct odb_source_files *)cb_data;
-	const struct odb_files_dir *dir = container_of(entry, const struct odb_files_dir, by_path_entry);
-	const char *path = keydata;
-
-	if (!path)
-		path = container_of(entry_or_key, const struct odb_files_dir, by_path_entry)->abspath;
-
-	return odb_files_dir_paths_cmp(files, dir->abspath, path);
-}
-
 /*
  * Return non-zero iff the path is usable as an alternate object directory.
  */
 static bool odb_files_dir_is_usable(struct odb_source_files *files,
 				    const char *path)
 {
-	struct strbuf normalized_objdir = STRBUF_INIT;
-	struct hashmap_entry key;
+	struct strbuf buf = STRBUF_INIT;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->base.path, 1);
-
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
 		error(_("object directory %s does not exist; "
@@ -239,28 +194,40 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	}
 
 	/*
-	 * Prevent the common mistake of listing the same
-	 * thing twice, or object directory itself.
+	 * Prevent the common mistake of listing the same thing twice, or
+	 * object directory itself.
 	 */
-	if (!hashmap_get_size(&files->dirs_by_path)) {
+	if (strset_empty(&files->dir_paths)) {
+		files->dir_paths_icase = 0;
+		repo_config_get_bool(files->base.odb->repo, "core.ignorecase",
+				     &files->dir_paths_icase);
+
+		strbuf_realpath(&buf, files->base.path, 1);
+		if (files->dir_paths_icase)
+			strbuf_tolower(&buf);
+		strset_add(&files->dir_paths, buf.buf);
+
 		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-			hashmap_entry_init(&dir->by_path_entry,
-					   strihash(dir->abspath));
-			hashmap_add(&files->dirs_by_path, &dir->by_path_entry);
+			strbuf_reset(&buf);
+			strbuf_addstr(&buf, dir->abspath);
+			if (files->dir_paths_icase)
+				strbuf_tolower(&buf);
+			strset_add(&files->dir_paths, buf.buf);
 		}
 	}
 
-	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
-		goto out;
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, path);
+	if (files->dir_paths_icase)
+		strbuf_tolower(&buf);
 
-	hashmap_entry_init(&key, strihash(path));
-	if (hashmap_get(&files->dirs_by_path, &key, path))
+	if (strset_contains(&files->dir_paths, buf.buf))
 		goto out;
 
 	usable = true;
 
 out:
-	strbuf_release(&normalized_objdir);
+	strbuf_release(&buf);
 	return usable;
 }
 
@@ -365,11 +332,17 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	*files->dirs_tail = alternate;
 	files->dirs_tail = &(alternate->next);
 
-	hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->abspath));
-	if (hashmap_get(&files->dirs_by_path, &alternate->by_path_entry,
-			alternate->abspath))
-		BUG("object directory must not yet exist");
-	hashmap_add(&files->dirs_by_path, &alternate->by_path_entry);
+	if (files->dir_paths_icase) {
+		struct strbuf path = STRBUF_INIT;
+
+		strbuf_addstr(&path, alternate->abspath);
+		strbuf_tolower(&path);
+		strset_add(&files->dir_paths, path.buf);
+
+		strbuf_release(&path);
+	} else {
+		strset_add(&files->dir_paths, alternate->abspath);
+	}
 
 	/* recursively add alternates */
 	read_alternates(alternate->abspath, &alternates);
@@ -1272,11 +1245,9 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	CALLOC_ARRAY(files, 1);
 	odb_source_init(&files->base, odb, ODB_SOURCE_FILES, object_dir);
 
-	hashmap_init(&files->dirs_by_path, odb_files_dir_by_path_cmp, files, 0);
-	files->dirs_paths_icase = -1;
-
 	files->dirs = odb_files_dir_new(odb, object_dir, true);
 	files->dirs_tail = &files->dirs->next;
+	strset_init(&files->dir_paths);
 
 	files->base.free = odb_source_files_free;
 	files->base.close = odb_source_files_close;
diff --git a/odb/source-files.h b/odb/source-files.h
index 837e890304..8695b6ee91 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "strmap.h"
 
 struct odb_source_loose;
 struct odb_source_packed;
@@ -18,12 +19,6 @@ struct odb_files_dir {
 	/* List of alternate object directories. */
 	struct odb_files_dir *next;
 
-	/*
-	 * Entry in the files source's map of directories, keyed by this
-	 * directory's path.
-	 */
-	struct hashmap_entry by_path_entry;
-
 	/* The two sources derived from this object directory. */
 	struct odb_source_loose *loose;
 	struct odb_source_packed *packed;
@@ -55,17 +50,17 @@ struct odb_source_files {
 	struct odb_files_dir **dirs_tail;
 
 	/*
-	 * Map of object directories, keyed by their respective paths. This
-	 * map is used to detect the case where the same directory is
-	 * registered multiple times.
+	 * Set of directory paths that have already been linked into this
+	 * source. This map is used to detect the case where the same directory
+	 * is registered multiple times.
 	 */
-	struct hashmap dirs_by_path;
+	struct strset dir_paths;
 
 	/*
 	 * Whether directory paths shall be compared case-insensitively, as
 	 * determined by "core.ignoreCase".
 	 */
-	int dirs_paths_icase;
+	int dir_paths_icase;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.307.g5fe7bd680c.dirty

[PATCH v2 5/6] odb/source-files: avoid stat'ing already-known alternates

[Patrick Steinhardt]

When adding alternates we first check whether the alternate exists
before checking whether we already know about it. This results in a
useless stat(3p) syscall in the case where we already track the
alternate.

Reorder the logic so that we first check whether we know the alternate
already before checking whether its directory exists to avoid the
syscall. Using the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.5 ms  ±     3.4 ms     285.0 ms  …   300.7 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    185.3 ms  ±     3.5 ms     181.2 ms  …   200.8 ms           -36.0%  (1.6x faster)
    Memory         5.7 MiB ±     0.0 MiB      5.5 MiB …     5.7 MiB           +0.2%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    269.2 ms  ±     4.5 ms     264.9 ms  …   290.0 ms            -7.0%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.9%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    260.7 ms  ±     3.4 ms     256.8 ms  …   271.1 ms           -10.0%
    Memory         5.9 MiB ±     0.1 MiB      5.8 MiB …     6.1 MiB           +4.7%

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 22e96828d2..f7eb4a9df7 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -185,14 +185,6 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct strbuf buf = STRBUF_INIT;
 	bool usable = false;
 
-	/* Detect cases where alternate disappeared */
-	if (!is_directory(path)) {
-		error(_("object directory %s does not exist; "
-			"check .git/objects/info/alternates"),
-		      path);
-		goto out;
-	}
-
 	/*
 	 * Prevent the common mistake of listing the same thing twice, or
 	 * object directory itself.
@@ -224,6 +216,14 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	if (strset_contains(&files->dir_paths, buf.buf))
 		goto out;
 
+	/* Detect cases where alternate disappeared */
+	if (!is_directory(path)) {
+		error(_("object directory %s does not exist; "
+			"check .git/objects/info/alternates"),
+		      path);
+		goto out;
+	}
+
 	usable = true;
 
 out:

-- 
2.56.0.307.g5fe7bd680c.dirty

[PATCH v2 6/6] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

In a preceding commit we have fixed a regression where we didn't re-read
the alternates file anymore when we hit a missing object. But while
correct, this change significantly hampers performance, even though
we're roughly at the same level compared to before the regression.

But the checks are unnecessarily expensive, as we always read and parse
the alternates file even in the case where it has not changed. This of
course translates into a lot of busywork for the common case, where the
"objects/info/alternates" is still the exact same.

Improve this by using a `struct stat_validity`. Like this we can easily
short-circuit reading the alternates file in case we can determine via a
stat(3p) call that the file hasn't changed. This significantly improves
performance:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    288.5 ms  ±     2.3 ms     284.9 ms  …   293.7 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    185.3 ms  ±     3.4 ms     181.2 ms  …   200.1 ms           -35.8%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    262.7 ms  ±     2.7 ms     257.1 ms  …   268.0 ms            -9.0%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.5%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    199.5 ms  ±     2.5 ms     195.4 ms  …   207.8 ms           -30.9%  (1.4x faster)
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.0 MiB           +4.3%

We're significantly faster compared to the original state now and are
almost as fast as the broken state where we don't reload alternates at
all. This is probably good enough for now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 39 ++++++++++++++++++++++++++++++---------
 odb/source-files.h |  4 ++++
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index f7eb4a9df7..6b4ac3e6be 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -73,6 +73,7 @@ static void odb_source_files_free(struct odb_source *source)
 		files->dirs = next;
 	}
 	strset_clear(&files->dir_paths);
+	stat_validity_clear(&files->alternates_validity);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -298,22 +299,41 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static int read_alternates(const char *object_dir, struct strvec *out)
+static void read_alternates(const char *object_dir, struct strvec *out,
+			    struct stat_validity *validity)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
 	path = xstrfmt("%s/info/alternates", object_dir);
-	if (strbuf_read_file(&buf, path, 1024) < 0) {
-		warn_on_fopen_errors(path);
-		free(path);
-		return 0;
+	if (!validity || !stat_validity_check(validity, path)) {
+		bool successful = false;
+		int fd;
+
+		fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			warn_on_fopen_errors(path);
+		} else if (strbuf_read(&buf, fd, 1024) < 0) {
+			warning_errno(_("unable to read alternates file '%s'"),
+				      path);
+		} else {
+			parse_alternates(buf.buf, '\n', object_dir, out);
+			successful = true;
+		}
+
+		if (validity) {
+			if (successful)
+				stat_validity_update(validity, fd);
+			else
+				stat_validity_clear(validity);
+		}
+
+		if (fd >= 0)
+			close(fd);
 	}
-	parse_alternates(buf.buf, '\n', object_dir, out);
 
 	strbuf_release(&buf);
 	free(path);
-	return 0;
 }
 
 static void add_alternate_recursively(struct odb_source_files *files,
@@ -345,7 +365,7 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	}
 
 	/* recursively add alternates */
-	read_alternates(alternate->abspath, &alternates);
+	read_alternates(alternate->abspath, &alternates, NULL);
 	if (alternates.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      path);
@@ -364,7 +384,8 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->base.path, &alternates);
+	read_alternates(files->base.path, &alternates,
+			&files->alternates_validity);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);
diff --git a/odb/source-files.h b/odb/source-files.h
index 8695b6ee91..de058edaed 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "statinfo.h"
 #include "strmap.h"
 
 struct odb_source_loose;
@@ -61,6 +62,9 @@ struct odb_source_files {
 	 * determined by "core.ignoreCase".
 	 */
 	int dir_paths_icase;
+
+	/* Stat data of the "info/alternates" file. */
+	struct stat_validity alternates_validity;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.307.g5fe7bd680c.dirty

Re: [PATCH v2 6/6] odb/source-files: only reload alternates when the file has changed

[Christian Couder]

On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:

> -static int read_alternates(const char *object_dir, struct strvec *out)
> +static void read_alternates(const char *object_dir, struct strvec *out,
> +                           struct stat_validity *validity)
>  {
>         struct strbuf buf = STRBUF_INIT;
>         char *path;
>
>         path = xstrfmt("%s/info/alternates", object_dir);
> -       if (strbuf_read_file(&buf, path, 1024) < 0) {
> -               warn_on_fopen_errors(path);
> -               free(path);
> -               return 0;
> +       if (!validity || !stat_validity_check(validity, path)) {
> +               bool successful = false;
> +               int fd;
> +
> +               fd = open(path, O_RDONLY);
> +               if (fd < 0) {
> +                       warn_on_fopen_errors(path);
> +               } else if (strbuf_read(&buf, fd, 1024) < 0) {

Here we start reading from the alternates file, but that file could
start to be written to...

> +                       warning_errno(_("unable to read alternates file '%s'"),
> +                                     path);
> +               } else {
> +                       parse_alternates(buf.buf, '\n', object_dir, out);

...suppose we parse the old content of the file (which calls
strbuf_realpath() for each entry and so does a few syscalls) while an
in-place rewrite of the file, like the:

  `echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates`

in the new t1006 test added by patch 3/6, happens...

> +                       successful = true;
> +               }
> +
> +               if (validity) {
> +                       if (successful)
> +                               stat_validity_update(validity, fd);

...then we would update the validity while the alternates file's
content has changed (and we only parsed its old content).

So, if the file content is not changed after that, we never parse the
content of the file from the in-place rewrite.

(If the rewrite is not an in-place one, but the file is for example
replaced through a lock file and rename, the race doesn't happen.)

It seems to me that if we want to avoid a race here, we should have
the `stat_validity_update(..., fd);` call before reading from fd like
in "refs/packed-backend.c" and "shallow.c".

> +                       else
> +                               stat_validity_clear(validity);
> +               }
> +
> +               if (fd >= 0)
> +                       close(fd);

It looks like v1 didn't have this issue, as it called
stat_validity_update() before reading. A concurrent rewrite then only
caused a spurious re-read on the next check. So I wonder if we could
keep that order, and just clear the validity when reading fails, which
would also address Junio's point:

               fd = open(path, O_RDONLY);
               if (fd < 0) {
                       warn_on_fopen_errors(path);
               } else {
                       if (validity)
                               stat_validity_update(validity, fd);
                       if (strbuf_read(&buf, fd, 1024) < 0) {
                               warning_errno(_("unable to read
alternates file '%s'"),
                                                       path);
                               if (validity)
                                       stat_validity_clear(validity);
                       } else {
                               parse_alternates(buf.buf, '\n', object_dir, out);
                       }
                       close(fd);
               }

Re: [PATCH v2 4/6] odb/source-files: simplify directory duplication detection

[Christian Couder]

On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:

> -/*
> - * NEEDSWORK: we're using "core.ignoreCase" to deduplicate alternates that
> - * _may_ be the same. This requires quite a bit of boilerplate for dubious
> - * benefit:
> - *
> - *   - Duplicating alternates should really only lead to regressed performance.
> - *
> - *   - We don't properly resolve symlinks or mointpoints, so we may still end
> - *     up duplicating alternates.
> - *
> - *   - The value may be lying, in which case we might deduplicate alternates
> - *     that are in fact not mapping to the same directory.
> - *
> - * We should investigate whether we can remove this whole mechanism outright.
> - */

Removing this NEEDSWORK isn't mentioned in the commit message. I agree
that the boilerplate is reduced with the new implementation, but I
think the points regarding mount points and a wrong `core.ignoreCase`
might still be interesting.

> @@ -55,17 +50,17 @@ struct odb_source_files {
>         struct odb_files_dir **dirs_tail;
>
>         /*
> -        * Map of object directories, keyed by their respective paths. This
> -        * map is used to detect the case where the same directory is
> -        * registered multiple times.
> +        * Set of directory paths that have already been linked into this
> +        * source. This map is used to detect the case where the same directory

s/map/set/

> +        * is registered multiple times.
>          */
> -       struct hashmap dirs_by_path;
> +       struct strset dir_paths;
>
>         /*
>          * Whether directory paths shall be compared case-insensitively, as
>          * determined by "core.ignoreCase".
>          */
> -       int dirs_paths_icase;
> +       int dir_paths_icase;

Nit: I guess this is to match the name of the strset, but it could
have been mentioned in the commit message.

Re: [PATCH v2 2/6] odb/source-files: always load alternates via primary object directory

[Christian Couder]

On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> The files source tracks a list of object directories. The head of the

Nit (to match 'the "files" source' used below): s/files/"files"/

> list is typically expected to be the primary object directory, whereas
> the tail is the list of alternates linked to it. This assumption does
> not always hold though, as transactions cause us to prepend another
> quarantine object directory to the list.
>
> This is not much of a problem now, but in a subsequent commit we're
> about to re-read "objects/info/alternates" when we hit a missing
> objects.

s/objects/object/

> So we may start calling `prepare_alternates()` in situations
> where the head of the list is _not_ the primary object directory, but if
> so we would now try to read alternates from the quarantine directory and
> not from the primary object directory.
>
> Fix this issue by instead using the path of the "files" source both when
> reading alternates in `prepare_alternates()` and when deduplicating the
> primary object directory in `odb_files_dir_is_usable()`.

Yeah, the change looks good to me.

Thanks.

Re: [PATCH v2 5/6] odb/source-files: avoid stat'ing already-known alternates

[Christian Couder]

On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> When adding alternates we first check whether the alternate exists
> before checking whether we already know about it. This results in a
> useless stat(3p) syscall in the case where we already track the
> alternate.
>
> Reorder the logic so that we first check whether we know the alternate
> already before checking whether its directory exists to avoid the
> syscall.

Does avoiding the syscall means not printing the "object directory %s
does not exist" error then, when we know the alternates but its
directory was removed?

Re: [PATCH v2 0/6] odb/files: reload alternates on retry again

[Christian Couder]

On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:

> In any case, this patch series fixes that. In practice, only the second
> commit is really needed to restore functionality. I was a bit unhappy
> about the performance impact of this change though, as fixing the logic
> regressed us from 180ms when looking up 20k missing objects to 309ms. So
> the last two patches do some optimizations that bring us down to 197ms.

It looks like some sentences in the above paragraph are still related
to the v1, especially:

- "only the second commit is really needed", and
- "the last two patches"

Thanks.

Re: [PATCH v2 4/6] odb/source-files: simplify directory duplication detection

[Patrick Steinhardt]

On Tue, Oct 27, 2026 at 01:13:49PM +0100, Christian Couder wrote:
> On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > -/*
> > - * NEEDSWORK: we're using "core.ignoreCase" to deduplicate alternates that
> > - * _may_ be the same. This requires quite a bit of boilerplate for dubious
> > - * benefit:
> > - *
> > - *   - Duplicating alternates should really only lead to regressed performance.
> > - *
> > - *   - We don't properly resolve symlinks or mointpoints, so we may still end
> > - *     up duplicating alternates.
> > - *
> > - *   - The value may be lying, in which case we might deduplicate alternates
> > - *     that are in fact not mapping to the same directory.
> > - *
> > - * We should investigate whether we can remove this whole mechanism outright.
> > - */
> 
> Removing this NEEDSWORK isn't mentioned in the commit message. I agree
> that the boilerplate is reduced with the new implementation, but I
> think the points regarding mount points and a wrong `core.ignoreCase`
> might still be interesting.

Yeah, that's a fair point. Will add it back in.

Patrick

Re: [PATCH v2 5/6] odb/source-files: avoid stat'ing already-known alternates

[Patrick Steinhardt]

On Tue, Oct 27, 2026 at 01:36:35PM +0100, Christian Couder wrote:
> On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > When adding alternates we first check whether the alternate exists
> > before checking whether we already know about it. This results in a
> > useless stat(3p) syscall in the case where we already track the
> > alternate.
> >
> > Reorder the logic so that we first check whether we know the alternate
> > already before checking whether its directory exists to avoid the
> > syscall.
> 
> Does avoiding the syscall means not printing the "object directory %s
> does not exist" error then, when we know the alternates but its
> directory was removed?

It depends. When trying to add an alternate that doesn't exist we'll
continue to print this error. But when such an alternate disappears
after we have already added it to the object database then we indeed
won't print this error anymore.

I don't think this really is a huge issue by itself, as it feels
sufficiently niche to not really matter, and the end result itself
wouldn't change. But let me know in case you disagree.

Patrick

Re: [PATCH v2 6/6] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

On Tue, Oct 27, 2026 at 12:22:50PM +0100, Christian Couder wrote:
> On Fri, Oct 23, 2026 at 2:48 PM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > -static int read_alternates(const char *object_dir, struct strvec *out)
> > +static void read_alternates(const char *object_dir, struct strvec *out,
> > +                           struct stat_validity *validity)
> >  {
> >         struct strbuf buf = STRBUF_INIT;
> >         char *path;
> >
> >         path = xstrfmt("%s/info/alternates", object_dir);
> > -       if (strbuf_read_file(&buf, path, 1024) < 0) {
> > -               warn_on_fopen_errors(path);
> > -               free(path);
> > -               return 0;
> > +       if (!validity || !stat_validity_check(validity, path)) {
> > +               bool successful = false;
> > +               int fd;
> > +
> > +               fd = open(path, O_RDONLY);
> > +               if (fd < 0) {
> > +                       warn_on_fopen_errors(path);
> > +               } else if (strbuf_read(&buf, fd, 1024) < 0) {
> 
> Here we start reading from the alternates file, but that file could
> start to be written to...
> 
> > +                       warning_errno(_("unable to read alternates file '%s'"),
> > +                                     path);
> > +               } else {
> > +                       parse_alternates(buf.buf, '\n', object_dir, out);
> 
> ...suppose we parse the old content of the file (which calls
> strbuf_realpath() for each entry and so does a few syscalls) while an
> in-place rewrite of the file, like the:
> 
>   `echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates`
> 
> in the new t1006 test added by patch 3/6, happens...
> 
> > +                       successful = true;
> > +               }
> > +
> > +               if (validity) {
> > +                       if (successful)
> > +                               stat_validity_update(validity, fd);
> 
> ...then we would update the validity while the alternates file's
> content has changed (and we only parsed its old content).
> 
> So, if the file content is not changed after that, we never parse the
> content of the file from the in-place rewrite.
> 
> (If the rewrite is not an in-place one, but the file is for example
> replaced through a lock file and rename, the race doesn't happen.)
> 
> It seems to me that if we want to avoid a race here, we should have
> the `stat_validity_update(..., fd);` call before reading from fd like
> in "refs/packed-backend.c" and "shallow.c".
> 
> > +                       else
> > +                               stat_validity_clear(validity);
> > +               }
> > +
> > +               if (fd >= 0)
> > +                       close(fd);
> 
> It looks like v1 didn't have this issue, as it called
> stat_validity_update() before reading. A concurrent rewrite then only
> caused a spurious re-read on the next check. So I wonder if we could
> keep that order, and just clear the validity when reading fails, which
> would also address Junio's point:
> 
>                fd = open(path, O_RDONLY);
>                if (fd < 0) {
>                        warn_on_fopen_errors(path);
>                } else {
>                        if (validity)
>                                stat_validity_update(validity, fd);
>                        if (strbuf_read(&buf, fd, 1024) < 0) {
>                                warning_errno(_("unable to read
> alternates file '%s'"),
>                                                        path);
>                                if (validity)
>                                        stat_validity_clear(validity);
>                        } else {
>                                parse_alternates(buf.buf, '\n', object_dir, out);
>                        }
>                        close(fd);
>                }

Hmm, that's fair. I don't think that we should worry too much about
in-place updates when concurrent processes are active as this is
inherently broken already. That process may see any in-between state,
and that may or may not result in something sensible. But fixing this is
easy enough.

Your proposed variant is subtly broken because you don't clear the stat
cache in the case where open(3p) itself fails. But I'll adapt that a bit
and then include this in the next version.

Thanks!

Patrick

Re: [PATCH v2 5/6] odb/source-files: avoid stat'ing already-known alternates

[Christian Couder]

On Tue, Oct 27, 2026 at 2:14 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 27, 2026 at 01:36:35PM +0100, Christian Couder wrote:

> > Does avoiding the syscall means not printing the "object directory %s
> > does not exist" error then, when we know the alternates but its
> > directory was removed?
>
> It depends. When trying to add an alternate that doesn't exist we'll
> continue to print this error. But when such an alternate disappears
> after we have already added it to the object database then we indeed
> won't print this error anymore.
>
> I don't think this really is a huge issue by itself, as it feels
> sufficiently niche to not really matter, and the end result itself
> wouldn't change. But let me know in case you disagree.

I don't disagree, but maybe the change deserves to be mentioned in the
commit message.

Thanks.

Re: [PATCH v2 5/6] odb/source-files: avoid stat'ing already-known alternates

[Patrick Steinhardt]

On Tue, Oct 27, 2026 at 03:31:43PM +0100, Christian Couder wrote:
> On Tue, Oct 27, 2026 at 2:14 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Oct 27, 2026 at 01:36:35PM +0100, Christian Couder wrote:
> 
> > > Does avoiding the syscall means not printing the "object directory %s
> > > does not exist" error then, when we know the alternates but its
> > > directory was removed?
> >
> > It depends. When trying to add an alternate that doesn't exist we'll
> > continue to print this error. But when such an alternate disappears
> > after we have already added it to the object database then we indeed
> > won't print this error anymore.
> >
> > I don't think this really is a huge issue by itself, as it feels
> > sufficiently niche to not really matter, and the end result itself
> > wouldn't change. But let me know in case you disagree.
> 
> I don't disagree, but maybe the change deserves to be mentioned in the
> commit message.

Makes sense, will do.

Patrick

[PATCH v3 0/6] odb/files: reload alternates on retry again

[Patrick Steinhardt]

Hi,

we recently noticed that we don't reload alternates anymore when an
object cannot be found. This used to be the case, but broke with
5946a564cd (odb/source: make `read_object_info()` function pluggable,
2026-03-05). And while it's a rather uncommon scenario that alternates
are getting rewritten while a Git process runs we managed to hit some
such scenarios in production systems.

In any case, this patch series fixes that. In practice, only the third
commit is really needed to restore functionality. I was a bit unhappy
about the performance impact of this change though, as fixing the logic
regressed us from 185ms when looking up 20k missing objects to 262ms. So
subsequent patches do some optimizations that bring us down to 199ms.

The series is built on top of 47fdf2208c (The 7th batch, 2026-10-19)
with ps/odb-files-alternates-cleanups at f211a3f0b5 (odb/source: drop
`local` flag, 2026-10-15) merged into it.

Changes in v3:
  - Reinstate NEEDSWORK comment.
  - Some typo fixes.
  - Update stat cache before reading the "info/alternates" again, while
    still invalidating it in case we hit an error.
  - Mention the change in behaviour around the error messages when an
    alternate disappears.
  - Link to v2: https://patch.msgid.link/20261023-pks-odb-files-reload-alternates-v2-0-30559d7ff43b@pks.im

Changes in v2:
  - We now properly load alternates from the primary object directory
    again even in the case where a quarantine directory is active, based
    on Christian's feedback.
  - I redid how we deduplicate alternates as a result of that. The
    resulting logic is simpler now while still giving the same
    guarantees as before.
  - Link to v1: https://patch.msgid.link/20261020-pks-odb-files-reload-alternates-v1-0-d9e156323836@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (6):
      odb/source-files: rename alternates-related functions
      odb/source-files: always load alternates via primary object directory
      odb/source-files: reload alternates on retry
      odb/source-files: simplify directory duplication detection
      odb/source-files: avoid stat'ing already-known alternates
      odb/source-files: only reload alternates when the file has changed

 odb/source-files.c  | 203 ++++++++++++++++++++++++++++------------------------
 odb/source-files.h  |  21 +++---
 t/t1006-cat-file.sh |  32 +++++++++
 3 files changed, 150 insertions(+), 106 deletions(-)

Range-diff versus v2:

1:  f7b3edcf250 = 1:  7979e39d4b7 odb/source-files: rename alternates-related functions
2:  3515b5acbe1 ! 2:  7c67df43102 odb/source-files: always load alternates via primary object directory
    @@ Metadata
      ## Commit message ##
         odb/source-files: always load alternates via primary object directory
     
    -    The files source tracks a list of object directories. The head of the
    +    The "files" source tracks a list of object directories. The head of the
         list is typically expected to be the primary object directory, whereas
         the tail is the list of alternates linked to it. This assumption does
         not always hold though, as transactions cause us to prepend another
         quarantine object directory to the list.
     
         This is not much of a problem now, but in a subsequent commit we're
    -    about to re-read "objects/info/alternates" when we hit a missing
    -    objects. So we may start calling `prepare_alternates()` in situations
    -    where the head of the list is _not_ the primary object directory, but if
    -    so we would now try to read alternates from the quarantine directory and
    -    not from the primary object directory.
    +    about to re-read "objects/info/alternates" when we hit a missing object.
    +    So we may start calling `prepare_alternates()` in situations where the
    +    head of the list is _not_ the primary object directory, but if so we
    +    would now try to read alternates from the quarantine directory and not
    +    from the primary object directory.
     
         Fix this issue by instead using the path of the "files" source both when
         reading alternates in `prepare_alternates()` and when deduplicating the
3:  da5a24624e8 = 3:  8882555124e odb/source-files: reload alternates on retry
4:  13e4e857f68 ! 4:  64d76b0ac94 odb/source-files: simplify directory duplication detection
    @@ Commit message
             Wall Time    266.5 ms  ±     2.8 ms     262.6 ms  …   276.7 ms            -7.8%
             Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +5.1%
     
    +    Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
    +    the newly added set.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## odb/source-files.c ##
    @@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files
      	/*
     -	 * Prevent the common mistake of listing the same
     -	 * thing twice, or object directory itself.
    -+	 * Prevent the common mistake of listing the same thing twice, or
    -+	 * object directory itself.
    ++	 * Prevent the common mistake of listing the same thing twice,
    ++	 * or object directory itself.
    ++	 *
    ++	 * NEEDSWORK: we're using "core.ignoreCase" to deduplicate
    ++	 * alternates that _may_ be the same. This requires quite a bit
    ++	 * of boilerplate for dubious benefit:
    ++	 *
    ++	 *   - Duplicating alternates should really only lead to
    ++	 *     regressed performance.
    ++	 *
    ++	 *   - We don't properly resolve symlinks or mointpoints, so we
    ++	 *     may still end up duplicating alternates.
    ++	 *
    ++	 *   - The value may be lying, in which case we might
    ++	 *     deduplicate alternates that are in fact not mapping to the
    ++	 *     same directory.
    ++	 *
    ++	 * We should investigate whether we can remove this whole
    ++	 * mechanism outright.
      	 */
     -	if (!hashmap_get_size(&files->dirs_by_path)) {
     +	if (strset_empty(&files->dir_paths)) {
    @@ odb/source-files.h: struct odb_source_files {
     -	 * map is used to detect the case where the same directory is
     -	 * registered multiple times.
     +	 * Set of directory paths that have already been linked into this
    -+	 * source. This map is used to detect the case where the same directory
    ++	 * source. This set is used to detect the case where the same directory
     +	 * is registered multiple times.
      	 */
     -	struct hashmap dirs_by_path;
5:  503d369bc82 ! 5:  387892c5b36 odb/source-files: avoid stat'ing already-known alternates
    @@ Commit message
             Wall Time    260.7 ms  ±     3.4 ms     256.8 ms  …   271.1 ms           -10.0%
             Memory         5.9 MiB ±     0.1 MiB      5.8 MiB …     6.1 MiB           +4.7%
     
    +    Note that this causes a user-visible change in behaviour: when an
    +    alternate was already linked into the "files" backend, its directory
    +    disappears afterwards and we later try to reload alternates, then we now
    +    stop printing the error message that the object directory does not
    +    exist. That feels sufficiently niche to not really matter in practice
    +    though, and other than that the user-observable behaviour remains the
    +    exact same.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## odb/source-files.c ##
    @@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files
     -	}
     -
      	/*
    - 	 * Prevent the common mistake of listing the same thing twice, or
    - 	 * object directory itself.
    + 	 * Prevent the common mistake of listing the same thing twice,
    + 	 * or object directory itself.
     @@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files *files,
      	if (strset_contains(&files->dir_paths, buf.buf))
      		goto out;
6:  8c4dbb5e7e2 ! 6:  1059c029cf7 odb/source-files: only reload alternates when the file has changed
    @@ odb/source-files.c: static void parse_alternates(const char *string,
     +		fd = open(path, O_RDONLY);
     +		if (fd < 0) {
     +			warn_on_fopen_errors(path);
    -+		} else if (strbuf_read(&buf, fd, 1024) < 0) {
    -+			warning_errno(_("unable to read alternates file '%s'"),
    -+				      path);
     +		} else {
    -+			parse_alternates(buf.buf, '\n', object_dir, out);
    -+			successful = true;
    -+		}
    -+
    -+		if (validity) {
    -+			if (successful)
    ++			if (validity)
     +				stat_validity_update(validity, fd);
    -+			else
    -+				stat_validity_clear(validity);
    ++			if (strbuf_read(&buf, fd, 1024) < 0) {
    ++				warning_errno(_("unable to read alternates file '%s'"),
    ++					      path);
    ++			} else {
    ++				parse_alternates(buf.buf, '\n', object_dir, out);
    ++				successful = true;
    ++			}
     +		}
     +
    ++		if (validity && !successful)
    ++			stat_validity_clear(validity);
     +		if (fd >= 0)
     +			close(fd);
      	}

---
base-commit: c8e565497cff47f6b23a6b96bef6fa82afa11667
change-id: 20261014-pks-odb-files-reload-alternates-dfc6dc010122

[PATCH v3 1/6] odb/source-files: rename alternates-related functions

[Patrick Steinhardt]

The functions that prepare and add alternates in the "files" backend
were originally part of the object database layer, which explains why
they still have an "odb_" prefix. That prefix does not make much sense
anymore now that those are part of the "files" backend.

Drop the prefix.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 5b4277fbfd3..d31da6e5d90 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -348,9 +348,9 @@ static int read_alternates(const char *object_dir, struct strvec *out)
 	return 0;
 }
 
-static void odb_add_alternate_recursively(struct odb_source_files *files,
-					  const char *path,
-					  int depth)
+static void add_alternate_recursively(struct odb_source_files *files,
+				      const char *path,
+				      int depth)
 {
 	struct odb_files_dir *alternate;
 	struct strvec alternates = STRVEC_INIT;
@@ -377,15 +377,15 @@ static void odb_add_alternate_recursively(struct odb_source_files *files,
 		      path);
 	} else {
 		for (size_t i = 0; i < alternates.nr; i++)
-			odb_add_alternate_recursively(files, alternates.v[i], depth + 1);
+			add_alternate_recursively(files, alternates.v[i], depth + 1);
 	}
 
  out:
 	strvec_clear(&alternates);
 }
 
-static void odb_prepare_alternates(struct odb_source_files *files,
-				   const char *alternate_db)
+static void prepare_alternates(struct odb_source_files *files,
+			       const char *alternate_db)
 {
 	struct strvec alternates = STRVEC_INIT;
 
@@ -393,7 +393,7 @@ static void odb_prepare_alternates(struct odb_source_files *files,
 	read_alternates(files->dirs->abspath, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(files, alternates.v[i], 0);
+		add_alternate_recursively(files, alternates.v[i], 0);
 
 	strvec_clear(&alternates);
 }
@@ -410,7 +410,7 @@ static void odb_source_files_prepare(struct odb_source *source,
 	 * for the lifetime of the process.
 	 */
 	if (flags & ODB_PREPARE_FLUSH_CACHES)
-		odb_prepare_alternates(files, NULL);
+		prepare_alternates(files, NULL);
 
 	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
 		odb_source_prepare(&dir->loose->base, flags);
@@ -1299,7 +1299,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	if (!is_absolute_path(object_dir))
 		chdir_notify_register(odb_source_files_reparent, files);
 
-	odb_prepare_alternates(files, alternates);
+	prepare_alternates(files, alternates);
 
 	free(object_dir);
 	free(alternates);

-- 
2.56.0.341.g393f52fd96.dirty

[PATCH v3 2/6] odb/source-files: always load alternates via primary object directory

[Patrick Steinhardt]

The "files" source tracks a list of object directories. The head of the
list is typically expected to be the primary object directory, whereas
the tail is the list of alternates linked to it. This assumption does
not always hold though, as transactions cause us to prepend another
quarantine object directory to the list.

This is not much of a problem now, but in a subsequent commit we're
about to re-read "objects/info/alternates" when we hit a missing object.
So we may start calling `prepare_alternates()` in situations where the
head of the list is _not_ the primary object directory, but if so we
would now try to read alternates from the quarantine directory and not
from the primary object directory.

Fix this issue by instead using the path of the "files" source both when
reading alternates in `prepare_alternates()` and when deduplicating the
primary object directory in `odb_files_dir_is_usable()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d31da6e5d90..c0375e7cb70 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -228,7 +228,7 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct hashmap_entry key;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->dirs->abspath, 1);
+	strbuf_realpath(&normalized_objdir, files->base.path, 1);
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
@@ -390,7 +390,7 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->dirs->abspath, &alternates);
+	read_alternates(files->base.path, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);

-- 
2.56.0.341.g393f52fd96.dirty

[PATCH v3 3/6] odb/source-files: reload alternates on retry

[Patrick Steinhardt]

When an object ID cannot be found we retry reading it from our packfiles
after we have flushed the respective caches of the packed backends. But
this misses one scenario: it may happen that the object databases'
alternates get rewritten concurrently and all of its objects are moved
to such an alternate. If so, we won't be able to find any of the objects
anymore. This used to be working just fine, but with 5946a564cd
(odb/source: make `read_object_info()` function pluggable, 2026-03-05)
we have accidentally dropped this logic.

This scenario sounds somewhat unlikely, but we've actually been hitting
it in the wild: when running long-lived git-cat-file(1) processes that
we interactively query over many minutes, and that repository gets
forked concurrently, then we move all objects into an object pool, write
a new alternates file, and the git-cat-file(1) process will remain
broken for the rest of its lifetime.

Fix this bug by re-preparing alternates when the object in question
could not be found and then retrying the read. A couple things to note:

  - This requires us to re-read loose objects, which is an operation
    that we previously skipped on the retry. But we only need to do this
    for the loose object directories that are actually new.

  - The result is still incomplete, as we only reload the alternates of
    the primary object directory. But alternates are added recursively,
    so in theory we'd also have to check whether alternates of any of
    the alternates have changed. But we never did that before the
    regression, either, so for now we don't care about this edge case.

  - It may happen that we reload when a quarantine directory is active.
    If no alternates existed before that reload we may not have lazily
    populated the map of object directories yet. So the assertion in
    `odb_files_dir_is_usable()` does not necessarily hold anymore, and
    we now have to populate the map with all object directories.

Furthermore, this change causes a performance regression when asking for
lots of missing objects. But that regression is basically in line with
the performance we had before this mechanism got broken. The following
benchmark demonstrates this when asking for 20k missing commits in an
empty repository with a single empty alternate:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.9 ms  ±     3.4 ms     285.8 ms  …   307.5 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    184.8 ms  ±     2.5 ms     181.5 ms  …   191.9 ms           -36.2%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    189.1 ms  ±     2.7 ms     185.9 ms  …   196.1 ms           -34.8%  (1.5x faster)
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.9%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    293.2 ms  ±     2.6 ms     289.3 ms  …   303.2 ms            +1.1%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.4%

There's no helping this though, with the regression fixed we will be
slower compared to before as we have to do more work. But the next
commits will improve the status quo a bit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c  | 21 +++++++++++++--------
 t/t1006-cat-file.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index c0375e7cb70..d66949b7de5 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -243,10 +243,11 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	 * thing twice, or object directory itself.
 	 */
 	if (!hashmap_get_size(&files->dirs_by_path)) {
-		assert(!files->dirs->next);
-		hashmap_entry_init(&files->dirs->by_path_entry,
-				   strihash(files->dirs->abspath));
-		hashmap_add(&files->dirs_by_path, &files->dirs->by_path_entry);
+		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
+			hashmap_entry_init(&dir->by_path_entry,
+					   strihash(dir->abspath));
+			hashmap_add(&files->dirs_by_path, &dir->by_path_entry);
+		}
 	}
 
 	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
@@ -425,6 +426,7 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 							      struct strbuf *errmsg)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct odb_files_dir **tail = NULL;
 	enum odb_read_status status = ODB_READ_NOT_FOUND;
 	enum odb_read_status ret;
 	bool retry = false;
@@ -448,10 +450,11 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 			status = ret;
 	}
 
-	for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-		if (retry)
-			break;
-
+	/*
+	 * When we have reprepared alternates then we only re-scan directories
+	 * that were newly added.
+	 */
+	for (struct odb_files_dir *dir = tail ? *tail : 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)
@@ -461,6 +464,8 @@ static enum odb_read_status odb_source_files_read_object_info(struct odb_source
 	}
 
 	if (!retry && !(flags & OBJECT_INFO_QUICK)) {
+		tail = files->dirs_tail;
+		prepare_alternates(files, NULL);
 		retry = true;
 		goto retry;
 	}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 72fc8acc611..f2b24dd2787 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -1481,4 +1481,36 @@ test_objects_filter "object:type=commit"
 test_objects_filter "object:type=tag"
 test_objects_filter "object:type=tree"
 
+test_expect_success PIPE 'concurrently written alternates are loaded' '
+	test_when_finished "rm -rf repo alternate stdin stdout" &&
+	git init repo &&
+	git init alternate &&
+	oid=$(git -C alternate hash-object -w --stdin </dev/null) &&
+	mkfifo stdin &&
+	mkfifo stdout &&
+	{ git -C repo cat-file --batch-check <stdin >stdout & } &&
+	(
+		exec 8>stdin &&
+		exec 9<stdout &&
+
+		# Read the object the first time, without having alternates
+		# wired up yet. The object should be reported as missing.
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid missing" >expect &&
+		test_cmp expect actual &&
+
+		# Read the object the second time, this time with the alternate
+		# set up. The backend should reload alternates and thus find
+		# the object.
+		echo "$PWD/alternate/.git/objects" >repo/.git/objects/info/alternates &&
+		echo "$oid" >&8 &&
+		read line <&9 &&
+		echo "$line" >actual &&
+		echo "$oid blob 0" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done

-- 
2.56.0.341.g393f52fd96.dirty

[PATCH v3 4/6] odb/source-files: simplify directory duplication detection

[Patrick Steinhardt]

When loading alternates it may happen that we try to add the same
alternate multiple times, either because "info/alternates" contains
duplicate entries or because one of its alternates matches the primary
object directory. We try to detect such duplicates via two mechanisms:

  - We have a hashmap of `struct odb_files_dir`, mapped by their
    absolute paths. If the path of an alternate is already present in
    this map we skip it.

  - We have another, explicit check to check whether the path of an
    alternate matches the primary object directory.

We can simplify both of these mechanisms by turning the map into a
strset, as we never access the values of the map. On the one hand this
gives us simpler interfaces to access the set. And on the other hand
this allows us to add two entries for the primary object directory: one
for the real path, and one for the absolute path.

Note that there is one gotcha though: when "core.ignorecase" is set then
the expectation is that we deduplicate alternates case-insensitively.
But we can also make that work rather easily by downcasing all paths we
add in that case.

Despite being significantly simpler, this also gives us a speedup
because we don't have to resolve the realpath of the primary object
directory on every call to `odb_files_dir_is_usable()` anymore. Using
the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.0 ms  ±     2.2 ms     285.8 ms  …   295.1 ms
    Memory         5.6 MiB ±     0.1 MiB      5.4 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    183.8 ms  ±     1.2 ms     181.5 ms  …   186.9 ms           -36.4%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.3 MiB …     5.7 MiB           -0.5%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    293.2 ms  ±     2.7 ms     288.8 ms  …   299.7 ms            +1.5%
    Memory         5.9 MiB ±     0.1 MiB      5.7 MiB …     6.1 MiB           +5.0%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    266.5 ms  ±     2.8 ms     262.6 ms  …   276.7 ms            -7.8%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +5.1%

Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
the newly added set.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 120 ++++++++++++++++++++++++-----------------------------
 odb/source-files.h |  17 +++-----
 2 files changed, 60 insertions(+), 77 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index d66949b7de5..8b63d0eff2b 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -72,7 +72,7 @@ static void odb_source_files_free(struct odb_source *source)
 		odb_files_dir_free(files->dirs);
 		files->dirs = next;
 	}
-	hashmap_clear(&files->dirs_by_path);
+	strset_clear(&files->dir_paths);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -176,60 +176,15 @@ static int odb_source_files_create_on_disk(struct odb_source *source,
 	return ret;
 }
 
-/*
- * NEEDSWORK: we're using "core.ignoreCase" to deduplicate alternates that
- * _may_ be the same. This requires quite a bit of boilerplate for dubious
- * benefit:
- *
- *   - Duplicating alternates should really only lead to regressed performance.
- *
- *   - We don't properly resolve symlinks or mointpoints, so we may still end
- *     up duplicating alternates.
- *
- *   - The value may be lying, in which case we might deduplicate alternates
- *     that are in fact not mapping to the same directory.
- *
- * We should investigate whether we can remove this whole mechanism outright.
- */
-static int odb_files_dir_paths_cmp(struct odb_source_files *files,
-				   const char *a, const char *b)
-{
-	if (files->dirs_paths_icase < 0) {
-		int icase = 0;
-		repo_config_get_bool(files->base.odb->repo, "core.ignorecase", &icase);
-		files->dirs_paths_icase = icase;
-	}
-
-	return files->dirs_paths_icase ? strcasecmp(a, b) : strcmp(a, b);
-}
-
-static int odb_files_dir_by_path_cmp(const void *cb_data,
-				     const struct hashmap_entry *entry,
-				     const struct hashmap_entry *entry_or_key,
-				     const void *keydata)
-{
-	struct odb_source_files *files = (struct odb_source_files *)cb_data;
-	const struct odb_files_dir *dir = container_of(entry, const struct odb_files_dir, by_path_entry);
-	const char *path = keydata;
-
-	if (!path)
-		path = container_of(entry_or_key, const struct odb_files_dir, by_path_entry)->abspath;
-
-	return odb_files_dir_paths_cmp(files, dir->abspath, path);
-}
-
 /*
  * Return non-zero iff the path is usable as an alternate object directory.
  */
 static bool odb_files_dir_is_usable(struct odb_source_files *files,
 				    const char *path)
 {
-	struct strbuf normalized_objdir = STRBUF_INIT;
-	struct hashmap_entry key;
+	struct strbuf buf = STRBUF_INIT;
 	bool usable = false;
 
-	strbuf_realpath(&normalized_objdir, files->base.path, 1);
-
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
 		error(_("object directory %s does not exist; "
@@ -239,28 +194,57 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	}
 
 	/*
-	 * Prevent the common mistake of listing the same
-	 * thing twice, or object directory itself.
+	 * Prevent the common mistake of listing the same thing twice,
+	 * or object directory itself.
+	 *
+	 * NEEDSWORK: we're using "core.ignoreCase" to deduplicate
+	 * alternates that _may_ be the same. This requires quite a bit
+	 * of boilerplate for dubious benefit:
+	 *
+	 *   - Duplicating alternates should really only lead to
+	 *     regressed performance.
+	 *
+	 *   - We don't properly resolve symlinks or mointpoints, so we
+	 *     may still end up duplicating alternates.
+	 *
+	 *   - The value may be lying, in which case we might
+	 *     deduplicate alternates that are in fact not mapping to the
+	 *     same directory.
+	 *
+	 * We should investigate whether we can remove this whole
+	 * mechanism outright.
 	 */
-	if (!hashmap_get_size(&files->dirs_by_path)) {
+	if (strset_empty(&files->dir_paths)) {
+		files->dir_paths_icase = 0;
+		repo_config_get_bool(files->base.odb->repo, "core.ignorecase",
+				     &files->dir_paths_icase);
+
+		strbuf_realpath(&buf, files->base.path, 1);
+		if (files->dir_paths_icase)
+			strbuf_tolower(&buf);
+		strset_add(&files->dir_paths, buf.buf);
+
 		for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) {
-			hashmap_entry_init(&dir->by_path_entry,
-					   strihash(dir->abspath));
-			hashmap_add(&files->dirs_by_path, &dir->by_path_entry);
+			strbuf_reset(&buf);
+			strbuf_addstr(&buf, dir->abspath);
+			if (files->dir_paths_icase)
+				strbuf_tolower(&buf);
+			strset_add(&files->dir_paths, buf.buf);
 		}
 	}
 
-	if (!odb_files_dir_paths_cmp(files, path, normalized_objdir.buf))
-		goto out;
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, path);
+	if (files->dir_paths_icase)
+		strbuf_tolower(&buf);
 
-	hashmap_entry_init(&key, strihash(path));
-	if (hashmap_get(&files->dirs_by_path, &key, path))
+	if (strset_contains(&files->dir_paths, buf.buf))
 		goto out;
 
 	usable = true;
 
 out:
-	strbuf_release(&normalized_objdir);
+	strbuf_release(&buf);
 	return usable;
 }
 
@@ -365,11 +349,17 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	*files->dirs_tail = alternate;
 	files->dirs_tail = &(alternate->next);
 
-	hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->abspath));
-	if (hashmap_get(&files->dirs_by_path, &alternate->by_path_entry,
-			alternate->abspath))
-		BUG("object directory must not yet exist");
-	hashmap_add(&files->dirs_by_path, &alternate->by_path_entry);
+	if (files->dir_paths_icase) {
+		struct strbuf path = STRBUF_INIT;
+
+		strbuf_addstr(&path, alternate->abspath);
+		strbuf_tolower(&path);
+		strset_add(&files->dir_paths, path.buf);
+
+		strbuf_release(&path);
+	} else {
+		strset_add(&files->dir_paths, alternate->abspath);
+	}
 
 	/* recursively add alternates */
 	read_alternates(alternate->abspath, &alternates);
@@ -1272,11 +1262,9 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	CALLOC_ARRAY(files, 1);
 	odb_source_init(&files->base, odb, ODB_SOURCE_FILES, object_dir);
 
-	hashmap_init(&files->dirs_by_path, odb_files_dir_by_path_cmp, files, 0);
-	files->dirs_paths_icase = -1;
-
 	files->dirs = odb_files_dir_new(odb, object_dir, true);
 	files->dirs_tail = &files->dirs->next;
+	strset_init(&files->dir_paths);
 
 	files->base.free = odb_source_files_free;
 	files->base.close = odb_source_files_close;
diff --git a/odb/source-files.h b/odb/source-files.h
index 837e8903044..595a4e78eef 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "strmap.h"
 
 struct odb_source_loose;
 struct odb_source_packed;
@@ -18,12 +19,6 @@ struct odb_files_dir {
 	/* List of alternate object directories. */
 	struct odb_files_dir *next;
 
-	/*
-	 * Entry in the files source's map of directories, keyed by this
-	 * directory's path.
-	 */
-	struct hashmap_entry by_path_entry;
-
 	/* The two sources derived from this object directory. */
 	struct odb_source_loose *loose;
 	struct odb_source_packed *packed;
@@ -55,17 +50,17 @@ struct odb_source_files {
 	struct odb_files_dir **dirs_tail;
 
 	/*
-	 * Map of object directories, keyed by their respective paths. This
-	 * map is used to detect the case where the same directory is
-	 * registered multiple times.
+	 * Set of directory paths that have already been linked into this
+	 * source. This set is used to detect the case where the same directory
+	 * is registered multiple times.
 	 */
-	struct hashmap dirs_by_path;
+	struct strset dir_paths;
 
 	/*
 	 * Whether directory paths shall be compared case-insensitively, as
 	 * determined by "core.ignoreCase".
 	 */
-	int dirs_paths_icase;
+	int dir_paths_icase;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.341.g393f52fd96.dirty

[PATCH v3 5/6] odb/source-files: avoid stat'ing already-known alternates

[Patrick Steinhardt]

When adding alternates we first check whether the alternate exists
before checking whether we already know about it. This results in a
useless stat(3p) syscall in the case where we already track the
alternate.

Reorder the logic so that we first check whether we know the alternate
already before checking whether its directory exists to avoid the
syscall. Using the same benchmark as in the preceding commit:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    289.5 ms  ±     3.4 ms     285.0 ms  …   300.7 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    185.3 ms  ±     3.5 ms     181.2 ms  …   200.8 ms           -36.0%  (1.6x faster)
    Memory         5.7 MiB ±     0.0 MiB      5.5 MiB …     5.7 MiB           +0.2%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    269.2 ms  ±     4.5 ms     264.9 ms  …   290.0 ms            -7.0%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.9%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    260.7 ms  ±     3.4 ms     256.8 ms  …   271.1 ms           -10.0%
    Memory         5.9 MiB ±     0.1 MiB      5.8 MiB …     6.1 MiB           +4.7%

Note that this causes a user-visible change in behaviour: when an
alternate was already linked into the "files" backend, its directory
disappears afterwards and we later try to reload alternates, then we now
stop printing the error message that the object directory does not
exist. That feels sufficiently niche to not really matter in practice
though, and other than that the user-observable behaviour remains the
exact same.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 8b63d0eff2b..4a2b9039784 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -185,14 +185,6 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	struct strbuf buf = STRBUF_INIT;
 	bool usable = false;
 
-	/* Detect cases where alternate disappeared */
-	if (!is_directory(path)) {
-		error(_("object directory %s does not exist; "
-			"check .git/objects/info/alternates"),
-		      path);
-		goto out;
-	}
-
 	/*
 	 * Prevent the common mistake of listing the same thing twice,
 	 * or object directory itself.
@@ -241,6 +233,14 @@ static bool odb_files_dir_is_usable(struct odb_source_files *files,
 	if (strset_contains(&files->dir_paths, buf.buf))
 		goto out;
 
+	/* Detect cases where alternate disappeared */
+	if (!is_directory(path)) {
+		error(_("object directory %s does not exist; "
+			"check .git/objects/info/alternates"),
+		      path);
+		goto out;
+	}
+
 	usable = true;
 
 out:

-- 
2.56.0.341.g393f52fd96.dirty

[PATCH v3 6/6] odb/source-files: only reload alternates when the file has changed

[Patrick Steinhardt]

In a preceding commit we have fixed a regression where we didn't re-read
the alternates file anymore when we hit a missing object. But while
correct, this change significantly hampers performance, even though
we're roughly at the same level compared to before the regression.

But the checks are unnecessarily expensive, as we always read and parse
the alternates file even in the case where it has not changed. This of
course translates into a lot of busywork for the common case, where the
"objects/info/alternates" is still the exact same.

Improve this by using a `struct stat_validity`. Like this we can easily
short-circuit reading the alternates file in case we can determine via a
stat(3p) call that the file hasn't changed. This significantly improves
performance:

  Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    288.5 ms  ±     2.3 ms     284.9 ms  …   293.7 ms
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB

  Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    185.3 ms  ±     3.4 ms     181.2 ms  …   200.1 ms           -35.8%  (1.6x faster)
    Memory         5.6 MiB ±     0.1 MiB      5.5 MiB …     5.7 MiB           -0.1%

  Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    262.7 ms  ±     2.7 ms     257.1 ms  …   268.0 ms            -9.0%
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +4.5%

  Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
                  mean     ±       σ          min     …     max
    Wall Time    199.5 ms  ±     2.5 ms     195.4 ms  …   207.8 ms           -30.9%  (1.4x faster)
    Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.0 MiB           +4.3%

We're significantly faster compared to the original state now and are
almost as fast as the broken state where we don't reload alternates at
all. This is probably good enough for now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-files.c | 38 +++++++++++++++++++++++++++++---------
 odb/source-files.h |  4 ++++
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/odb/source-files.c b/odb/source-files.c
index 4a2b9039784..5f5e6bfd7d8 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -73,6 +73,7 @@ static void odb_source_files_free(struct odb_source *source)
 		files->dirs = next;
 	}
 	strset_clear(&files->dir_paths);
+	stat_validity_clear(&files->alternates_validity);
 
 	odb_source_release(&files->base);
 	free(files);
@@ -315,22 +316,40 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static int read_alternates(const char *object_dir, struct strvec *out)
+static void read_alternates(const char *object_dir, struct strvec *out,
+			    struct stat_validity *validity)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
 	path = xstrfmt("%s/info/alternates", object_dir);
-	if (strbuf_read_file(&buf, path, 1024) < 0) {
-		warn_on_fopen_errors(path);
-		free(path);
-		return 0;
+	if (!validity || !stat_validity_check(validity, path)) {
+		bool successful = false;
+		int fd;
+
+		fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			warn_on_fopen_errors(path);
+		} else {
+			if (validity)
+				stat_validity_update(validity, fd);
+			if (strbuf_read(&buf, fd, 1024) < 0) {
+				warning_errno(_("unable to read alternates file '%s'"),
+					      path);
+			} else {
+				parse_alternates(buf.buf, '\n', object_dir, out);
+				successful = true;
+			}
+		}
+
+		if (validity && !successful)
+			stat_validity_clear(validity);
+		if (fd >= 0)
+			close(fd);
 	}
-	parse_alternates(buf.buf, '\n', object_dir, out);
 
 	strbuf_release(&buf);
 	free(path);
-	return 0;
 }
 
 static void add_alternate_recursively(struct odb_source_files *files,
@@ -362,7 +381,7 @@ static void add_alternate_recursively(struct odb_source_files *files,
 	}
 
 	/* recursively add alternates */
-	read_alternates(alternate->abspath, &alternates);
+	read_alternates(alternate->abspath, &alternates, NULL);
 	if (alternates.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      path);
@@ -381,7 +400,8 @@ static void prepare_alternates(struct odb_source_files *files,
 	struct strvec alternates = STRVEC_INIT;
 
 	parse_alternates(alternate_db, PATH_SEP, NULL, &alternates);
-	read_alternates(files->base.path, &alternates);
+	read_alternates(files->base.path, &alternates,
+			&files->alternates_validity);
 
 	for (size_t i = 0; i < alternates.nr; i++)
 		add_alternate_recursively(files, alternates.v[i], 0);
diff --git a/odb/source-files.h b/odb/source-files.h
index 595a4e78eef..99217bc0a53 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -2,6 +2,7 @@
 #define ODB_SOURCE_FILES_H
 
 #include "odb/source.h"
+#include "statinfo.h"
 #include "strmap.h"
 
 struct odb_source_loose;
@@ -61,6 +62,9 @@ struct odb_source_files {
 	 * determined by "core.ignoreCase".
 	 */
 	int dir_paths_icase;
+
+	/* Stat data of the "info/alternates" file. */
+	struct stat_validity alternates_validity;
 };
 
 /* Allocate and initialize a new object source. */

-- 
2.56.0.341.g393f52fd96.dirty

Re: [PATCH v3 4/6] odb/source-files: simplify directory duplication detection

[Christian Couder]

On Wed, Oct 28, 2026 at 8:11 AM Patrick Steinhardt <ps@pks.im> wrote:

> Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of

Nit: s/dir_paths_iscase/dir_paths_icase/

Otherwise the whole v3 looks good to me. Thanks.

> the newly added set.

Re: [PATCH v3 4/6] odb/source-files: simplify directory duplication detection

[Patrick Steinhardt]

On Wed, Oct 28, 2026 at 09:59:57AM +0100, Christian Couder wrote:
> On Wed, Oct 28, 2026 at 8:11 AM Patrick Steinhardt <ps@pks.im> wrote:
> 
> > Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
> 
> Nit: s/dir_paths_iscase/dir_paths_icase/
> 
> Otherwise the whole v3 looks good to me. Thanks.

D'oh. To many s's that tripped me here. I've queued this locally, but
will refrain from sending a new version just to fix this one commit
message typo.

Thanks for your review!

Patrick

Re: [PATCH v3 0/6] odb/files: reload alternates on retry again

[Junio C Hamano]

H
Patrick Steinhardt <ps@pks.im> writes:

> 3:  da5a24624e8 = 3:  8882555124e odb/source-files: reload alternates on retry
> 4:  13e4e857f68 ! 4:  64d76b0ac94 odb/source-files: simplify directory duplication detection
>     @@ Commit message
>              Wall Time    266.5 ms  ±     2.8 ms     262.6 ms  …   276.7 ms            -7.8%
>              Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +5.1%
>      
>     +    Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
>     +    the newly added set.
>     +
>          Signed-off-by: Patrick Steinhardt <ps@pks.im>
>      
>       ## odb/source-files.c ##
>     @@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files
>       	/*
>      -	 * Prevent the common mistake of listing the same
>      -	 * thing twice, or object directory itself.
>     -+	 * Prevent the common mistake of listing the same thing twice, or
>     -+	 * object directory itself.
>     ++	 * Prevent the common mistake of listing the same thing twice,
>     ++	 * or object directory itself.
>     ++	 *
>     ++	 * NEEDSWORK: we're using "core.ignoreCase" to deduplicate
>     ++	 * alternates that _may_ be the same. This requires quite a bit
>     ++	 * of boilerplate for dubious benefit:
>     ++	 *
>     ++	 *   - Duplicating alternates should really only lead to
>     ++	 *     regressed performance.
>     ++	 *
>     ++	 *   - We don't properly resolve symlinks or mointpoints, so we
>     ++	 *     may still end up duplicating alternates.

Good thought process.  "Mountpoint" is misspelled, by the way.

>     ++	 *
>     ++	 *   - The value may be lying, in which case we might
>     ++	 *     deduplicate alternates that are in fact not mapping to the
>     ++	 *     same directory.

This one is much more serious than the previous two.  As an
optimization to avoid repeating work known to produce no new
results, deduplication can tolerate false negatives (e.g., we
thought "DIR" and "Dir" were different and ended up scanning the
same thing twice).  It cannot, however, allow false positives, which
would lead to "missing" objects that only the mistakenly
deduplicated object store could have provided.

>     ++	 * We should investigate whether we can remove this whole
>     ++	 * mechanism outright.
>       	 */

Sure.

I guess the previous round was already almost ready with Christian's
review, and this round addressing his comments, we are pretty much
done with the topic and can mark it as ready for 'next'?

Thanks, all.

Re: [PATCH v3 4/6] odb/source-files: simplify directory duplication detection

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Oct 28, 2026 at 09:59:57AM +0100, Christian Couder wrote:
>> On Wed, Oct 28, 2026 at 8:11 AM Patrick Steinhardt <ps@pks.im> wrote:
>> 
>> > Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
>> 
>> Nit: s/dir_paths_iscase/dir_paths_icase/
>> 
>> Otherwise the whole v3 looks good to me. Thanks.
>
> D'oh. To many s's that tripped me here. I've queued this locally, but
> will refrain from sending a new version just to fix this one commit
> message typo.
>
> Thanks for your review!
>
> Patrick

If you have a single list of less than 7 lines to fix up, I can do so
locally before applying.

Thanks, all.

Re: [PATCH v3 0/6] odb/files: reload alternates on retry again

[Patrick Steinhardt]

On Wed, Oct 28, 2026 at 09:12:47AM -0700, Junio C Hamano wrote:
> H
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 3:  da5a24624e8 = 3:  8882555124e odb/source-files: reload alternates on retry
> > 4:  13e4e857f68 ! 4:  64d76b0ac94 odb/source-files: simplify directory duplication detection
> >     @@ Commit message
> >              Wall Time    266.5 ms  ±     2.8 ms     262.6 ms  …   276.7 ms            -7.8%
> >              Memory         5.9 MiB ±     0.1 MiB      5.6 MiB …     6.1 MiB           +5.1%
> >      
> >     +    Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
> >     +    the newly added set.
> >     +
> >          Signed-off-by: Patrick Steinhardt <ps@pks.im>
> >      
> >       ## odb/source-files.c ##
> >     @@ odb/source-files.c: static bool odb_files_dir_is_usable(struct odb_source_files
> >       	/*
> >      -	 * Prevent the common mistake of listing the same
> >      -	 * thing twice, or object directory itself.
> >     -+	 * Prevent the common mistake of listing the same thing twice, or
> >     -+	 * object directory itself.
> >     ++	 * Prevent the common mistake of listing the same thing twice,
> >     ++	 * or object directory itself.
> >     ++	 *
> >     ++	 * NEEDSWORK: we're using "core.ignoreCase" to deduplicate
> >     ++	 * alternates that _may_ be the same. This requires quite a bit
> >     ++	 * of boilerplate for dubious benefit:
> >     ++	 *
> >     ++	 *   - Duplicating alternates should really only lead to
> >     ++	 *     regressed performance.
> >     ++	 *
> >     ++	 *   - We don't properly resolve symlinks or mointpoints, so we
> >     ++	 *     may still end up duplicating alternates.
> 
> Good thought process.  "Mountpoint" is misspelled, by the way.
> 
> >     ++	 *
> >     ++	 *   - The value may be lying, in which case we might
> >     ++	 *     deduplicate alternates that are in fact not mapping to the
> >     ++	 *     same directory.
> 
> This one is much more serious than the previous two.  As an
> optimization to avoid repeating work known to produce no new
> results, deduplication can tolerate false negatives (e.g., we
> thought "DIR" and "Dir" were different and ended up scanning the
> same thing twice).  It cannot, however, allow false positives, which
> would lead to "missing" objects that only the mistakenly
> deduplicated object store could have provided.
> 
> >     ++	 * We should investigate whether we can remove this whole
> >     ++	 * mechanism outright.
> >       	 */
> 
> Sure.
> 
> I guess the previous round was already almost ready with Christian's
> review, and this round addressing his comments, we are pretty much
> done with the topic and can mark it as ready for 'next'?

Yeah, I think it should be ready to go by now. Thanks!

Patrick