{"thread":{"id":"66146","subject":"[PATCH 0/4] odb: eagerly load alternates","startedAt":"2026-08-10T13:33:40Z","lastAt":"2026-08-21T15:19:39Z","messageCount":47,"participants":["Patrick Steinhardt","Justin Tobler","Junio C Hamano","Karthik Nayak","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"550181","messageId":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","threadId":"66146","inReplyTo":null,"subject":"[PATCH 0/4] odb: eagerly load alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T13:33:27Z","receivedAt":"2026-08-10T13:33:40Z","isPatch":true,"body":"Hi,\n\nwhen initializing the object database we only eagerly initialize the\nprimary object database source. If the primary source has alternates,\nthose alternates are only initialized the first time we really access\nthe object database.\n\nWhen introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to\nsupport multiple object databases., 2005-05-07), alternates were\noriginally only loaded when a given object wasn't found in the primary\nobject database. This was also reinforced by later optimization, for\nexample in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,\n2007-05-26), where we tried to avoid loading alternates in even more\ncases. But as Git has evolved, we eventually started to eagerly parse\nalternates all over the codebase, including on every single object\nlookup, and consequently deferring this operation does not really buy us\nmuch anymore.\n\nThe result of this is that we have calls to `odb_prepare_alternates()`\ncluttered all over the code base. This is somewhat awkward, and as\nalmost every Git command ends up reading objects at it doesn't even buy\nus anything.\n\nThis patch series thus gets rid of the lazy-loading. Besides simplifying\nthe codebase a bit, it also prepares us for moving alternates into the\n\"files\" backend as discussed in [1].\n\nThe series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\nwith ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\non-disk structures pluggable, 2026-08-07) merged into it.\n\nThanks!\n\nPatrick\n\n[1]: <amLgMqkqxR8mKIbT@pks.im>\n\n---\nPatrick Steinhardt (4):\n      odb: decouple source path comparisons from `the_repository`\n      odb: eagerly initialize alternates\n      odb: drop `loaded_alternates` field\n      odb: drop `alternates_db` field\n\n builtin/fsck.c         |   3 --\n builtin/pack-objects.c |   3 --\n commit-graph.c         |   4 --\n loose.c                |   1 -\n object-name.c          |   1 -\n odb.c                  | 106 ++++++++++++++++++++++++-------------------------\n odb.h                  |  22 +++++-----\n odb/source.h           |   7 ++++\n odb/streaming.c        |   1 -\n pack-bitmap.c          |   2 -\n packfile.c             |   1 -\n packfile.h             |   2 -\n 12 files changed, 68 insertions(+), 85 deletions(-)\n\n\n---\nbase-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765\nchange-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd\n\n"},{"id":"550182","messageId":"20260810-pks-odb-eagerly-prepare-alternates-v1-1-f0fa4a4004e1@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T13:33:28Z","receivedAt":"2026-08-10T13:33:41Z","isPatch":true,"body":"When registering alternates we deduplicate object database sources by\ntheir path so that the same source won't be added twice. Ever since\ncf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\nthis duplicate check is backed by a map keyed by the source's path,\nusing `fspathhash()` and `fspatheq()` as hash and equality functions,\nrespectively.\n\nThese functions are problematic in this context for two reasons:\n\n  - They implicitly depend on `the_repository` instead of the\n    repository that owns the object database.\n\n  - They derive case-sensitivity from `repo_ignore_case()`, which\n    returns a default value in case the repository's configuration has\n    not been parsed yet. Object database sources may be registered\n    before that is the case, so the answer may flip depending on when a\n    source gets registered.\n\nFix this by making the comparison self-contained in the object\ndatabase. Instead of using `fspathhash()` and `fspatheq()` we resolve\n\"core.ignoreCase\" manually and then use the correct comparison function\nbased on the result. This requires us to migrate to a `struct hashmap`,\nas the khash interface does not give us the ability to change these\nfunctions.\n\nNote that we can unconditionally use `strihash()` to compute entry\nhashes regardless of case sensitivity: a hash function only needs to\nguarantee that equal keys have equal hashes, and a case-insensitive\nhash satisfies this requirement for both case-sensitive and\ncase-insensitive equality.\n\nOverall it's quite debatable whether all of this complexity really is\nworth it, or whether we should just linearly search through all sources\nto find duplicates. But the mentioned commit cares about cases with\nthousands of alternates, and a linear search would of course regress\nperformance quite a bit. This doesn't really feel like a reasonable case\nto care about though, but I don't feel comfortable regressing it anyway.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c        | 63 ++++++++++++++++++++++++++++++++++++++++--------------------\n odb.h        | 15 ++++++++++++++-\n odb/source.h |  7 +++++++\n 3 files changed, 63 insertions(+), 22 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex bd02d8ad54..51da386f22 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -2,11 +2,10 @@\n #include \"abspath.h\"\n #include \"commit-graph.h\"\n #include \"config.h\"\n-#include \"dir.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n+#include \"hashmap.h\"\n #include \"hex.h\"\n-#include \"khash.h\"\n #include \"lockfile.h\"\n #include \"loose.h\"\n #include \"midx.h\"\n@@ -29,8 +28,32 @@\n #include \"trace2.h\"\n #include \"write-or-die.h\"\n \n-KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n-\tstruct odb_source *, 1, fspathhash, fspatheq)\n+static int odb_source_paths_cmp(struct object_database *o,\n+\t\t\t\tconst char *a, const char *b)\n+{\n+\tif (o->source_paths_icase < 0) {\n+\t\tint icase = 0;\n+\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n+\t\to->source_paths_icase = icase;\n+\t}\n+\n+\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n+}\n+\n+static int odb_source_by_path_cmp(const void *cb_data,\n+\t\t\t\t  const struct hashmap_entry *entry,\n+\t\t\t\t  const struct hashmap_entry *entry_or_key,\n+\t\t\t\t  const void *keydata)\n+{\n+\tstruct object_database *o = (struct object_database *)cb_data;\n+\tconst struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry);\n+\tconst char *path = keydata;\n+\n+\tif (!path)\n+\t\tpath = container_of(entry_or_key, const struct odb_source, by_path_entry)->path;\n+\n+\treturn odb_source_paths_cmp(o, source->path, path);\n+}\n \n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern)\n@@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb,\n  */\n static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n-\tint r;\n \tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tstruct hashmap_entry key;\n \tbool usable = false;\n \n \tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n@@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \t * Prevent the common mistake of listing the same\n \t * thing twice, or object directory itself.\n \t */\n-\tif (!o->source_by_path) {\n-\t\tkhiter_t p;\n-\n-\t\to->source_by_path = kh_init_odb_path_map();\n+\tif (!hashmap_get_size(&o->source_by_path)) {\n \t\tassert(!o->sources->next);\n-\t\tp = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r);\n-\t\tassert(r == 1); /* never used */\n-\t\tkh_value(o->source_by_path, p) = o->sources;\n+\t\thashmap_entry_init(&o->sources->by_path_entry,\n+\t\t\t\t   strihash(o->sources->path));\n+\t\thashmap_add(&o->source_by_path, &o->sources->by_path_entry);\n \t}\n \n-\tif (fspatheq(path, normalized_objdir.buf))\n+\tif (!odb_source_paths_cmp(o, path, normalized_objdir.buf))\n \t\tgoto out;\n \n-\tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n+\thashmap_entry_init(&key, strihash(path));\n+\tif (hashmap_get(&o->source_by_path, &key, path))\n \t\tgoto out;\n \n \tusable = true;\n@@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n {\n \tstruct odb_source *alternate = NULL;\n \tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n \n \tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n@@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t*odb->sources_tail = alternate;\n \todb->sources_tail = &(alternate->next);\n \n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n+\thashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path));\n+\tif (hashmap_get(&odb->source_by_path, &alternate->by_path_entry,\n+\t\t\talternate->path))\n \t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n+\thashmap_add(&odb->source_by_path, &alternate->by_path_entry);\n \n \t/* recursively add alternates */\n \todb_source_read_alternates(alternate, &sources);\n@@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->repo = repo;\n \tpthread_mutex_init(&o->replace_mutex, NULL);\n \tstring_list_init_dup(&o->submodule_source_paths);\n+\thashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);\n+\to->source_paths_icase = -1;\n \n \tif (flags & ODB_NEW_HONOR_ENV) {\n \t\tprimary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT));\n@@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o)\n \todb_source_free(o->inmemory_objects);\n \to->inmemory_objects = NULL;\n \n-\tkh_destroy_odb_path_map(o->source_by_path);\n-\to->source_by_path = NULL;\n+\thashmap_clear(&o->source_by_path);\n }\n \n void odb_free(struct object_database *o)\ndiff --git a/odb.h b/odb.h\nindex 8eb4e85d64..71af7450a9 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_H\n #define ODB_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"oidset.h\"\n #include \"oidmap.h\"\n@@ -54,7 +55,19 @@ struct object_database {\n \t */\n \tstruct odb_source *sources;\n \tstruct odb_source **sources_tail;\n-\tstruct kh_odb_path_map *source_by_path;\n+\n+\t/*\n+\t * Map of object database sources, keyed by their respective paths.\n+\t * This map is used to detect the case where the same source is\n+\t * registered multiple times.\n+\t */\n+\tstruct hashmap source_by_path;\n+\n+\t/*\n+\t * Whether source paths shall be compared case-insensitively, as\n+\t * determined by \"core.ignoreCase\".\n+\t */\n+\tint source_paths_icase;\n \n \tint loaded_alternates;\n \ndiff --git a/odb/source.h b/odb/source.h\nindex 4bc037b8d6..82cda8ad75 100644\n--- a/odb/source.h\n+++ b/odb/source.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_SOURCE_H\n #define ODB_SOURCE_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -50,6 +51,12 @@ struct strvec;\n struct odb_source {\n \tstruct odb_source *next;\n \n+\t/*\n+\t * Entry in the object database's map of sources, keyed by this\n+\t * source's path.\n+\t */\n+\tstruct hashmap_entry by_path_entry;\n+\n \t/* Object database that owns this object source. */\n \tstruct object_database *odb;\n \n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550183","messageId":"20260810-pks-odb-eagerly-prepare-alternates-v1-2-f0fa4a4004e1@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH 2/4] odb: eagerly initialize alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T13:33:29Z","receivedAt":"2026-08-10T13:33:44Z","isPatch":true,"body":"When creating the object database we initialize the main object database\nsource, but we don't yet initialize its alternates. Instead, we have\nmany calls to `odb_prepare_alternates()` cluttered around the code base\nwhenever we are about to iterate through the sources.\n\nThis lazy loading doesn't really add much value: the moment where read\nany object we _have_ to load the alternates anyway. So given that most\nof our commands would access the object database this optimization is\nnot really buying us much in the first place. Quite on the contrary, it\nmakes the code harder to understand and is a potential source of bugs in\ncase any callsite forgot to prepare alternates before we iterate through\nthe sources.\n\nHistorically though there was a reason why we deferred lazy-loading: it\nmay happen that the repository has \"core.ignoreCase\" configured, and we\nuse that to deduplicate the list of alternates in case we had the same\nalternate configured multiple times, but with different casing. We used\nto initialize the object database before we had fully configured the\nowning repository though, and consequently we couldn't access that\nconfiguration yet. This has changed in the preceding commit though where\nwe started to parse \"core.ignoreCase\" manually.\n\nEagerly prepare alternates both when creating the object database and\nwhen flushing its caches. Drop the now-unneeded calls to prepare the\nalternates that are scattered across the code base.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fsck.c         |  3 ---\n builtin/pack-objects.c |  3 ---\n commit-graph.c         |  4 ----\n loose.c                |  1 -\n object-name.c          |  1 -\n odb.c                  | 26 ++++----------------------\n odb.h                  |  6 ------\n odb/streaming.c        |  1 -\n pack-bitmap.c          |  2 --\n packfile.c             |  1 -\n packfile.h             |  2 --\n 11 files changed, 4 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex a6c054e45b..892c5661d9 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1069,7 +1069,6 @@ int cmd_fsck(int argc,\n \t\todb_for_each_object(repo->objects, NULL,\n \t\t\t\t    mark_object_for_connectivity, repo, 0);\n \t} else {\n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next)\n \t\t\tfsck_source(repo, source);\n \n@@ -1155,7 +1154,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_commit_graph) {\n \t\tstruct child_process commit_graph_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&commit_graph_verify);\n \t\t\tcommit_graph_verify.git_cmd = 1;\n@@ -1173,7 +1171,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_multi_pack_index) {\n \t\tstruct child_process midx_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&midx_verify);\n \t\t\tmidx_verify.git_cmd = 1;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ec5b6f206..48d37e8e32 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1779,8 +1779,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid,\n \t\t*found_offset = 0;\n \t}\n \n-\todb_prepare_alternates(the_repository->objects);\n-\n \tfor (source = the_repository->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\n@@ -4520,7 +4518,6 @@ static void add_objects_in_unpacked_packs(void)\n \t\t.source_infop = &source_info,\n \t};\n \n-\todb_prepare_alternates(to_pack.repo->objects);\n \tfor (source = to_pack.repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \ndiff --git a/commit-graph.c b/commit-graph.c\nindex 49e8f63930..983c11ce85 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -651,8 +651,6 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb,\n \tcount = st->st_size / (odb->repo->hash_algo->hexsz + 1);\n \tCALLOC_ARRAY(oids, count);\n \n-\todb_prepare_alternates(odb);\n-\n \tfor (i = 0; i < count; i++) {\n \t\tstruct odb_source *source;\n \n@@ -768,7 +766,6 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n \tif (!commit_graph_compatible(r))\n \t\treturn NULL;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tr->objects->commit_graph = read_commit_graph_one(source);\n \t\tif (r->objects->commit_graph)\n@@ -2018,7 +2015,6 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)\n \t\t\t_(\"Finding commits for commit graph among packed objects\"),\n \t\t\tctx->approx_nr_objects);\n \n-\todb_prepare_alternates(ctx->r->objects);\n \tfor (source = ctx->r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\todb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi,\ndiff --git a/loose.c b/loose.c\nindex aa3cb1b4fc..c159d29d2d 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -115,7 +115,6 @@ int repo_read_loose_object_map(struct repository *repo)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(repo->objects);\n \tfor (source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (loose_object_map_load(files->loose) < 0)\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..34a08d76dd 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -280,7 +280,6 @@ static int init_object_disambiguation(struct repository *r,\n \n \tds->len = len;\n \tds->repo = r;\n-\todb_prepare_alternates(r->objects);\n \treturn 0;\n }\n \ndiff --git a/odb.c b/odb.c\nindex 51da386f22..2ae8228dd2 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -237,11 +237,6 @@ void odb_add_to_alternates_file(struct object_database *odb,\n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t\t\t\t\t\tconst char *dir)\n {\n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n \treturn odb_add_alternate_recursively(odb, dir, 0);\n }\n \n@@ -250,12 +245,6 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n {\n \tstruct odb_source *source;\n \n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n-\n \t/*\n \t * Make a new primary odb and link the old primary ODB in as an\n \t * alternate\n@@ -361,7 +350,6 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_\n \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n \tstruct strbuf odb_path_real = STRBUF_INIT;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tstrbuf_realpath(&odb_path_real, source->path, 1);\n \t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n@@ -495,7 +483,6 @@ int odb_for_each_alternate(struct object_database *odb,\n \tstruct odb_source *alternate;\n \tint r = 0;\n \n-\todb_prepare_alternates(odb);\n \tfor (alternate = odb->sources->next; alternate; alternate = alternate->next) {\n \t\tr = cb(alternate, payload);\n \t\tif (r)\n@@ -504,7 +491,7 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n@@ -523,7 +510,6 @@ void odb_prepare_alternates(struct object_database *odb)\n \n int odb_has_alternates(struct object_database *odb)\n {\n-\todb_prepare_alternates(odb);\n \treturn !!odb->sources->next;\n }\n \n@@ -583,8 +569,6 @@ static int do_oid_object_info_extended(struct object_database *odb,\n \tif (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags))\n \t\treturn 0;\n \n-\todb_prepare_alternates(odb);\n-\n \twhile (1) {\n \t\tstruct odb_source *source;\n \n@@ -847,7 +831,6 @@ int odb_freshen_object(struct object_database *odb,\n \t\t       const struct object_id *oid)\n {\n \tstruct odb_source *source;\n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (odb_source_freshen_object(source, oid, NULL))\n \t\t\treturn 1;\n@@ -862,7 +845,6 @@ int odb_for_each_object_ext(struct object_database *odb,\n {\n \tint ret;\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tif (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local)\n \t\t\tcontinue;\n@@ -900,7 +882,6 @@ int odb_count_objects(struct object_database *odb,\n \t\treturn 0;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tunsigned long c;\n \n@@ -980,7 +961,6 @@ int odb_find_abbrev_len(struct object_database *odb,\n \t\tgoto out;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tret = odb_source_find_abbrev_len(source, oid, len, &len);\n \t\tif (ret)\n@@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n+\todb_prepare_alternates(o);\n+\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n \t\to->loaded_alternates = 0;\n+\t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\n \n-\todb_prepare_alternates(o);\n \tfor (source = o->sources; source; source = source->next)\n \t\todb_source_prepare(source, flags);\n \ndiff --git a/odb.h b/odb.h\nindex 71af7450a9..fbafee174b 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -273,12 +273,6 @@ void odb_for_each_alternate_ref(struct object_database *odb,\n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern);\n \n-/*\n- * Prepare alternate object sources for the given database by reading\n- * \"objects/info/alternates\" and opening the respective sources.\n- */\n-void odb_prepare_alternates(struct object_database *odb);\n-\n /*\n  * Check whether the object database has any alternates. The primary object\n  * source does not count as alternate.\ndiff --git a/odb/streaming.c b/odb/streaming.c\nindex 20531e864c..37642768e9 100644\n--- a/odb/streaming.c\n+++ b/odb/streaming.c\n@@ -184,7 +184,6 @@ static int istream_source(struct odb_read_stream **out,\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (!odb_source_read_object_stream(out, source, oid))\n \t\t\treturn 0;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e85bd69ba4..e0fb57d332 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -717,7 +717,6 @@ static int open_bitmap(struct repository *r,\n \n \tassert(!bitmap_git->map);\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \n@@ -3417,7 +3416,6 @@ int verify_bitmap_files(struct repository *r)\n \tstruct packed_git *p;\n \tint res = 0;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\ndiff --git a/packfile.c b/packfile.c\nindex 0eee45055f..d870de90ed 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1938,7 +1938,6 @@ int has_object_pack(struct repository *r, const struct object_id *oid)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0))\ndiff --git a/packfile.h b/packfile.h\nindex e1f77152b5..10de24f477 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -77,8 +77,6 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct\n {\n \tstruct repo_for_each_pack_data data = { 0 };\n \n-\todb_prepare_alternates(repo->objects);\n-\n \tfor (struct odb_source *source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct packfile_list_entry *entry = packfile_store_get_packs(files->packed);\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550184","messageId":"20260810-pks-odb-eagerly-prepare-alternates-v1-3-f0fa4a4004e1@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH 3/4] odb: drop `loaded_alternates` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T13:33:30Z","receivedAt":"2026-08-10T13:33:47Z","isPatch":true,"body":"The `struct object_database::loaded_alternates` field tells us whether\nor not alternates have been loaded already. This field was useful before\nthe preceding commit as we were indeed lazy-loading alternates. But now\nthat we started to eagerly load them we can assume them to be loaded\nafter `odb_new()`, and hence the field does not serve any purpose\nanymore.\n\nRemove it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 9 +--------\n odb.h | 2 --\n 2 files changed, 1 insertion(+), 10 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 2ae8228dd2..2eb37a2f44 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \tint ret = odb_source_write_alternate(odb->sources, dir);\n \tif (ret < 0)\n \t\tdie(NULL);\n-\tif (odb->loaded_alternates)\n-\t\todb_add_alternate_recursively(odb, dir, 0);\n+\todb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n@@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tif (odb->loaded_alternates)\n-\t\treturn;\n-\n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n-\todb->loaded_alternates = 1;\n-\n \tstrvec_clear(&sources);\n }\n \n@@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * the lifetime of the process.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\to->loaded_alternates = 0;\n \t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\ndiff --git a/odb.h b/odb.h\nindex fbafee174b..aefb34213f 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,8 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\tint loaded_alternates;\n-\n \t/*\n \t * A list of alternate object directories loaded from the environment;\n \t * this should not generally need to be accessed directly, but will\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550185","messageId":"20260810-pks-odb-eagerly-prepare-alternates-v1-4-f0fa4a4004e1@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH 4/4] odb: drop `alternates_db` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T13:33:31Z","receivedAt":"2026-08-10T13:33:50Z","isPatch":true,"body":"The `struct object_database::alternates_db` field tracks the value of\nthe \"GIT_ALTERNATE_OBJECT_DIRECTORIES\" environment variable and is\nused in `odb_prepare_alternates()`. It's not necessary to store it as a\nseparate field anymore though, as we stopped lazy-loading alternates.\nConsequently, we can simply pass it to `odb_prepare_alternates()` via\n`odb_new()` now.\n\nDo so and remove the field.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 14 +++++++-------\n odb.h |  7 -------\n 2 files changed, 7 insertions(+), 14 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 2eb37a2f44..fc21199f80 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -490,12 +490,14 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-static void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb,\n+\t\t\t\t   const char *alternate_db)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tparse_alternates(alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n+\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n@@ -1062,11 +1064,11 @@ struct object_database *odb_new(struct repository *repo,\n \n \to->sources = odb_source_new(o, primary_source, true);\n \to->sources_tail = &o->sources->next;\n-\to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n-\todb_prepare_alternates(o);\n+\todb_prepare_alternates(o, secondary_sources);\n \n+\tfree(secondary_sources);\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1100,8 +1102,6 @@ void odb_free(struct object_database *o)\n \tif (!o)\n \t\treturn;\n \n-\tfree(o->alternate_db);\n-\n \toidmap_clear(&o->replace_map, 1);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n@@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * the lifetime of the process.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\todb_prepare_alternates(o);\n+\t\todb_prepare_alternates(o, NULL);\n \t\to->object_count_valid = 0;\n \t}\n \ndiff --git a/odb.h b/odb.h\nindex aefb34213f..748366a610 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,13 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\t/*\n-\t * A list of alternate object directories loaded from the environment;\n-\t * this should not generally need to be accessed directly, but will\n-\t * populate the \"sources\" list when odb_prepare_alternates() is run.\n-\t */\n-\tchar *alternate_db;\n-\n \t/*\n \t * Objects that should be substituted by other objects\n \t * (see git-replace(1)).\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550331","messageId":"anuP0Mh9aBz9VdBK@denethor","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-1-f0fa4a4004e1@pks.im","subject":"Re: [PATCH 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-08-11T22:04:58Z","receivedAt":"2026-08-11T22:05:07Z","isPatch":true,"body":"On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> When registering alternates we deduplicate object database sources by\n> their path so that the same source won't be added twice. Ever since\n> cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> this duplicate check is backed by a map keyed by the source's path,\n> using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> respectively.\n> \n> These functions are problematic in this context for two reasons:\n> \n>   - They implicitly depend on `the_repository` instead of the\n>     repository that owns the object database.\n> \n>   - They derive case-sensitivity from `repo_ignore_case()`, which\n>     returns a default value in case the repository's configuration has\n>     not been parsed yet. Object database sources may be registered\n>     before that is the case, so the answer may flip depending on when a\n>     source gets registered.\n\nAre alternates currently always registered after repository\nconfiguration has been parsed? Or is this an existing bug?\n\n> Fix this by making the comparison self-contained in the object\n> database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n> \"core.ignoreCase\" manually and then use the correct comparison function\n> based on the result. This requires us to migrate to a `struct hashmap`,\n> as the khash interface does not give us the ability to change these\n> functions.\n> \n> Note that we can unconditionally use `strihash()` to compute entry\n> hashes regardless of case sensitivity: a hash function only needs to\n> guarantee that equal keys have equal hashes, and a case-insensitive\n> hash satisfies this requirement for both case-sensitive and\n> case-insensitive equality.\n\nOk IIUC, even if we want to be case-sensitive, its ok to use\n`strihash()` and have hash collisions because the compare function will\nstill properly distinguish between the cases. Makes sense.\n\n> Overall it's quite debatable whether all of this complexity really is\n> worth it, or whether we should just linearly search through all sources\n> to find duplicates. But the mentioned commit cares about cases with\n> thousands of alternates, and a linear search would of course regress\n> performance quite a bit. This doesn't really feel like a reasonable case\n> to care about though, but I don't feel comfortable regressing it anyway.\n\nYa, my first though here was also whether all of this song and dance is\nreally needed for alternates. There may be someone out there with tons\nof alternates I guess though. Probably good to be on the safe side.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c        | 63 ++++++++++++++++++++++++++++++++++++++++--------------------\n>  odb.h        | 15 ++++++++++++++-\n>  odb/source.h |  7 +++++++\n>  3 files changed, 63 insertions(+), 22 deletions(-)\n> \n> diff --git a/odb.c b/odb.c\n> index bd02d8ad54..51da386f22 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -2,11 +2,10 @@\n>  #include \"abspath.h\"\n>  #include \"commit-graph.h\"\n>  #include \"config.h\"\n> -#include \"dir.h\"\n>  #include \"environment.h\"\n>  #include \"gettext.h\"\n> +#include \"hashmap.h\"\n>  #include \"hex.h\"\n> -#include \"khash.h\"\n>  #include \"lockfile.h\"\n>  #include \"loose.h\"\n>  #include \"midx.h\"\n> @@ -29,8 +28,32 @@\n>  #include \"trace2.h\"\n>  #include \"write-or-die.h\"\n>  \n> -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n> -\tstruct odb_source *, 1, fspathhash, fspatheq)\n> +static int odb_source_paths_cmp(struct object_database *o,\n> +\t\t\t\tconst char *a, const char *b)\n> +{\n> +\tif (o->source_paths_icase < 0) {\n> +\t\tint icase = 0;\n> +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n> +\t\to->source_paths_icase = icase;\n> +\t}\n\nWe now parse ignorecase configuration here directly and store the result\nin `source_paths_icase`. This ensures configuration is correctly applied\nregardless of whether repository configuration has been fully read yet.\n\n> +\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n> +}\n> +\n> +static int odb_source_by_path_cmp(const void *cb_data,\n> +\t\t\t\t  const struct hashmap_entry *entry,\n> +\t\t\t\t  const struct hashmap_entry *entry_or_key,\n> +\t\t\t\t  const void *keydata)\n> +{\n> +\tstruct object_database *o = (struct object_database *)cb_data;\n> +\tconst struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry);\n> +\tconst char *path = keydata;\n> +\n> +\tif (!path)\n> +\t\tpath = container_of(entry_or_key, const struct odb_source, by_path_entry)->path;\n> +\n> +\treturn odb_source_paths_cmp(o, source->path, path);\n> +}\n\nHere is the comparison callback that is used for the hashmap.\n\n>  int odb_mkstemp(struct object_database *odb,\n>  \t\tstruct strbuf *temp_filename, const char *pattern)\n> @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb,\n>   */\n>  static bool odb_is_source_usable(struct object_database *o, const char *path)\n>  {\n> -\tint r;\n>  \tstruct strbuf normalized_objdir = STRBUF_INIT;\n> +\tstruct hashmap_entry key;\n>  \tbool usable = false;\n>  \n>  \tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n> @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n>  \t * Prevent the common mistake of listing the same\n>  \t * thing twice, or object directory itself.\n>  \t */\n> -\tif (!o->source_by_path) {\n> -\t\tkhiter_t p;\n> -\n> -\t\to->source_by_path = kh_init_odb_path_map();\n> +\tif (!hashmap_get_size(&o->source_by_path)) {\n>  \t\tassert(!o->sources->next);\n> -\t\tp = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r);\n> -\t\tassert(r == 1); /* never used */\n> -\t\tkh_value(o->source_by_path, p) = o->sources;\n> +\t\thashmap_entry_init(&o->sources->by_path_entry,\n> +\t\t\t\t   strihash(o->sources->path));\n> +\t\thashmap_add(&o->source_by_path, &o->sources->by_path_entry);\n\nThe hashmap is lazily set up with the primary source. I do find some of\nthe variable names like \"source_by_path\" a bit vague, but that isn't\nreally anything new here.\n\n>  \t}\n>  \n> -\tif (fspatheq(path, normalized_objdir.buf))\n> +\tif (!odb_source_paths_cmp(o, path, normalized_objdir.buf))\n>  \t\tgoto out;\n\nIf the path matches the first entry in the sources list then we know it\nis not an alternate.\n\n>  \n> -\tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n> +\thashmap_entry_init(&key, strihash(path));\n> +\tif (hashmap_get(&o->source_by_path, &key, path))\n>  \t\tgoto out;\n\nIf the alternates source cannot be found for the given path, then we\nalso know it is not a usuable alternate.\n\n>  \tusable = true;\n> @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n>  {\n>  \tstruct odb_source *alternate = NULL;\n>  \tstruct strvec sources = STRVEC_INIT;\n> -\tkhiter_t pos;\n> -\tint ret;\n>  \n>  \tif (!odb_is_source_usable(odb, source))\n>  \t\tgoto error;\n> @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n>  \t*odb->sources_tail = alternate;\n>  \todb->sources_tail = &(alternate->next);\n>  \n> -\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n> -\tif (!ret)\n> +\thashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path));\n> +\tif (hashmap_get(&odb->source_by_path, &alternate->by_path_entry,\n> +\t\t\talternate->path))\n>  \t\tBUG(\"source must not yet exist\");\n> -\tkh_value(odb->source_by_path, pos) = alternate;\n> +\thashmap_add(&odb->source_by_path, &alternate->by_path_entry);\n\nHere is where alternates get registered and added to the hashmap. Makes\nsense.\n\nOverall this patch looks good.\n\n-Justin\n"},{"id":"550332","messageId":"anucxvBIF-5Wmd33@denethor","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-2-f0fa4a4004e1@pks.im","subject":"Re: [PATCH 2/4] odb: eagerly initialize alternates","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-08-11T22:15:33Z","receivedAt":"2026-08-11T22:15:37Z","isPatch":true,"body":"On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> When creating the object database we initialize the main object database\n> source, but we don't yet initialize its alternates. Instead, we have\n> many calls to `odb_prepare_alternates()` cluttered around the code base\n> whenever we are about to iterate through the sources.\n> \n> This lazy loading doesn't really add much value: the moment where read\n\nShould this say \"where we read\" instead?\n\n> any object we _have_ to load the alternates anyway. So given that most\n> of our commands would access the object database this optimization is\n> not really buying us much in the first place. Quite on the contrary, it\n> makes the code harder to understand and is a potential source of bugs in\n> case any callsite forgot to prepare alternates before we iterate through\n> the sources.\n> \n> Historically though there was a reason why we deferred lazy-loading: it\n> may happen that the repository has \"core.ignoreCase\" configured, and we\n> use that to deduplicate the list of alternates in case we had the same\n> alternate configured multiple times, but with different casing. We used\n> to initialize the object database before we had fully configured the\n> owning repository though, and consequently we couldn't access that\n> configuration yet. This has changed in the preceding commit though where\n> we started to parse \"core.ignoreCase\" manually.\n> \n> Eagerly prepare alternates both when creating the object database and\n> when flushing its caches. Drop the now-unneeded calls to prepare the\n> alternates that are scattered across the code base.\n\nThis sounds like the right direction and overall much simpler. Nice.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n[snip]\n> @@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo,\n>  \to->alternate_db = secondary_sources;\n>  \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n>  \n> +\todb_prepare_alternates(o);\n\nNow we eagerly prepare alternates at time of ODB creation.\n\n> +\n>  \tfree(primary_source);\n>  \treturn o;\n>  }\n> @@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n>  \t */\n>  \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n>  \t\to->loaded_alternates = 0;\n> +\t\todb_prepare_alternates(o);\n>  \t\to->object_count_valid = 0;\n>  \t}\n\nOk we also invoke `odb_prepare_alternates()` when we need to refresh all\nalternate sources. Makes sense.\n\nThe rest of this patch just removes the now-unneeded\n`odb_prepare_alternates()` call sites and looks good.\n\n-Justin\n"},{"id":"550334","messageId":"anufdy4UAqoLWPgG@denethor","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-3-f0fa4a4004e1@pks.im","subject":"Re: [PATCH 3/4] odb: drop `loaded_alternates` field","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-08-11T22:22:28Z","receivedAt":"2026-08-11T22:22:31Z","isPatch":true,"body":"On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> The `struct object_database::loaded_alternates` field tells us whether\n> or not alternates have been loaded already. This field was useful before\n> the preceding commit as we were indeed lazy-loading alternates. But now\n> that we started to eagerly load them we can assume them to be loaded\n> after `odb_new()`, and hence the field does not serve any purpose\n> anymore.\n\nNow that alternates are eagerly set up, it is safe to assume, if we have\nan ODB, the alternates have been loaded. Makes sense.\n\n> Remove it.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n[snip]\n> @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n>  \t * the lifetime of the process.\n>  \t */\n>  \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n> -\t\to->loaded_alternates = 0;\n>  \t\todb_prepare_alternates(o);\n\nAlso nice to see this go away as I thought it was little bit awkward to\nunset it just to allow the us to reprepare the alternates.\n\n-Justin\n"},{"id":"550335","messageId":"anug-cxSSsy45swy@denethor","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-4-f0fa4a4004e1@pks.im","subject":"Re: [PATCH 4/4] odb: drop `alternates_db` field","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-08-11T22:31:17Z","receivedAt":"2026-08-11T22:31:19Z","isPatch":true,"body":"On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> The `struct object_database::alternates_db` field tracks the value of\n> the \"GIT_ALTERNATE_OBJECT_DIRECTORIES\" environment variable and is\n> used in `odb_prepare_alternates()`. It's not necessary to store it as a\n> separate field anymore though, as we stopped lazy-loading alternates.\n> Consequently, we can simply pass it to `odb_prepare_alternates()` via\n> `odb_new()` now.\n> \n> Do so and remove the field.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n[snip]\n> @@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n>  \t * the lifetime of the process.\n>  \t */\n>  \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n> -\t\todb_prepare_alternates(o);\n> +\t\todb_prepare_alternates(o, NULL);\n>  \t\to->object_count_valid = 0;\n>  \t}\n\nNaive question: is the reason we don't need to wire the\n`GIT_ALTERNATE_OBJECT_DIRECTORIES` environment variable here because\nthey have already been added as sources? IOW, when we invoke\n`odb_prepare_alternates()` after the initial set up, we only really care\nabout re-reading the alternates file.\n\n-Justin\n"},{"id":"550338","messageId":"anwG_yIy0eNsZi2n@pks.im","threadId":"66146","inReplyTo":"anuP0Mh9aBz9VdBK@denethor","subject":"Re: [PATCH 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T05:39:11Z","receivedAt":"2026-08-12T05:39:19Z","isPatch":true,"body":"On Tue, Aug 11, 2026 at 05:04:58PM -0500, Justin Tobler wrote:\n> On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> > When registering alternates we deduplicate object database sources by\n> > their path so that the same source won't be added twice. Ever since\n> > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> > this duplicate check is backed by a map keyed by the source's path,\n> > using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> > respectively.\n> > \n> > These functions are problematic in this context for two reasons:\n> > \n> >   - They implicitly depend on `the_repository` instead of the\n> >     repository that owns the object database.\n> > \n> >   - They derive case-sensitivity from `repo_ignore_case()`, which\n> >     returns a default value in case the repository's configuration has\n> >     not been parsed yet. Object database sources may be registered\n> >     before that is the case, so the answer may flip depending on when a\n> >     source gets registered.\n> \n> Are alternates currently always registered after repository\n> configuration has been parsed? Or is this an existing bug?\n\nThey are, because of the lazy-loading. So this is not a bug, we merely\nhave to ensure that we retain this behaviour.\n\n> > Overall it's quite debatable whether all of this complexity really is\n> > worth it, or whether we should just linearly search through all sources\n> > to find duplicates. But the mentioned commit cares about cases with\n> > thousands of alternates, and a linear search would of course regress\n> > performance quite a bit. This doesn't really feel like a reasonable case\n> > to care about though, but I don't feel comfortable regressing it anyway.\n> \n> Ya, my first though here was also whether all of this song and dance is\n> really needed for alternates. There may be someone out there with tons\n> of alternates I guess though. Probably good to be on the safe side.\n\ncf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\nmentions a repository with 100k alternates in total, but that's an\nartificial testing setup. I doubt you can get any kind of reasonable\nperformance out of such a repository, regardless of whether on not\nparsing the alternates is going to be fast.\n\nFor now though I didn't want to remove this infra. It feels overblown,\nbut it's not an unmaintainable mess, either.\n\nPatrick\n"},{"id":"550339","messageId":"anwHB3rWh7YLK7ky@pks.im","threadId":"66146","inReplyTo":"anucxvBIF-5Wmd33@denethor","subject":"Re: [PATCH 2/4] odb: eagerly initialize alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T05:39:19Z","receivedAt":"2026-08-12T05:39:24Z","isPatch":true,"body":"On Tue, Aug 11, 2026 at 05:15:33PM -0500, Justin Tobler wrote:\n> On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> > When creating the object database we initialize the main object database\n> > source, but we don't yet initialize its alternates. Instead, we have\n> > many calls to `odb_prepare_alternates()` cluttered around the code base\n> > whenever we are about to iterate through the sources.\n> > \n> > This lazy loading doesn't really add much value: the moment where read\n> \n> Should this say \"where we read\" instead?\n\nYes, indeed.\n\nPatrick\n"},{"id":"550340","messageId":"anwHDe5PfAaT1k9W@pks.im","threadId":"66146","inReplyTo":"anug-cxSSsy45swy@denethor","subject":"Re: [PATCH 4/4] odb: drop `alternates_db` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T05:39:25Z","receivedAt":"2026-08-12T05:39:30Z","isPatch":true,"body":"On Tue, Aug 11, 2026 at 05:31:17PM -0500, Justin Tobler wrote:\n> On 26/08/10 03:33PM, Patrick Steinhardt wrote:\n> > The `struct object_database::alternates_db` field tracks the value of\n> > the \"GIT_ALTERNATE_OBJECT_DIRECTORIES\" environment variable and is\n> > used in `odb_prepare_alternates()`. It's not necessary to store it as a\n> > separate field anymore though, as we stopped lazy-loading alternates.\n> > Consequently, we can simply pass it to `odb_prepare_alternates()` via\n> > `odb_new()` now.\n> > \n> > Do so and remove the field.\n> > \n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> [snip]\n> > @@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n> >  \t * the lifetime of the process.\n> >  \t */\n> >  \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n> > -\t\todb_prepare_alternates(o);\n> > +\t\todb_prepare_alternates(o, NULL);\n> >  \t\to->object_count_valid = 0;\n> >  \t}\n> \n> Naive question: is the reason we don't need to wire the\n> `GIT_ALTERNATE_OBJECT_DIRECTORIES` environment variable here because\n> they have already been added as sources? IOW, when we invoke\n> `odb_prepare_alternates()` after the initial set up, we only really care\n> about re-reading the alternates file.\n\nYes, exactly. We set up alternates exactly once in `odb_new()`, and we\ndon't expect the environment variable to ever change in a running\nprocess. And as `odb_prepare_alternates()` only adds but never removes\nany it's fine to ignore those here.\n\nI'll add a comment.\n\nPatrick\n"},{"id":"550383","messageId":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH v2 0/4] odb: eagerly load alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T09:13:56Z","receivedAt":"2026-08-12T09:14:10Z","isPatch":true,"body":"Hi,\n\nwhen initializing the object database we only eagerly initialize the\nprimary object database source. If the primary source has alternates,\nthose alternates are only initialized the first time we really access\nthe object database.\n\nWhen introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to\nsupport multiple object databases., 2005-05-07), alternates were\noriginally only loaded when a given object wasn't found in the primary\nobject database. This was also reinforced by later optimization, for\nexample in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,\n2007-05-26), where we tried to avoid loading alternates in even more\ncases. But as Git has evolved, we eventually started to eagerly parse\nalternates all over the codebase, including on every single object\nlookup, and consequently deferring this operation does not really buy us\nmuch anymore.\n\nThe result of this is that we have calls to `odb_prepare_alternates()`\ncluttered all over the code base. This is somewhat awkward, and as\nalmost every Git command ends up reading objects at it doesn't even buy\nus anything.\n\nThis patch series thus gets rid of the lazy-loading. Besides simplifying\nthe codebase a bit, it also prepares us for moving alternates into the\n\"files\" backend as discussed in [1].\n\nThe series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\nwith ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\non-disk structures pluggable, 2026-08-07) merged into it.\n\nChanges in v2:\n  - Add a missing word to a commit message.\n  - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES\n    when re-preparing the object database.\n  - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <amLgMqkqxR8mKIbT@pks.im>\n\n---\nPatrick Steinhardt (4):\n      odb: decouple source path comparisons from `the_repository`\n      odb: eagerly initialize alternates\n      odb: drop `loaded_alternates` field\n      odb: drop `alternates_db` field\n\n builtin/fsck.c         |   3 --\n builtin/pack-objects.c |   3 --\n commit-graph.c         |   4 --\n loose.c                |   1 -\n object-name.c          |   1 -\n odb.c                  | 109 ++++++++++++++++++++++++-------------------------\n odb.h                  |  22 +++++-----\n odb/source.h           |   7 ++++\n odb/streaming.c        |   1 -\n pack-bitmap.c          |   2 -\n packfile.c             |   1 -\n packfile.h             |   2 -\n 12 files changed, 70 insertions(+), 86 deletions(-)\n\nRange-diff versus v1:\n\n1:  25802adffa = 1:  721907c60d odb: decouple source path comparisons from `the_repository`\n2:  1e73b730d8 ! 2:  3b2c23566c odb: eagerly initialize alternates\n    @@ Commit message\n         many calls to `odb_prepare_alternates()` cluttered around the code base\n         whenever we are about to iterate through the sources.\n     \n    -    This lazy loading doesn't really add much value: the moment where read\n    -    any object we _have_ to load the alternates anyway. So given that most\n    -    of our commands would access the object database this optimization is\n    -    not really buying us much in the first place. Quite on the contrary, it\n    -    makes the code harder to understand and is a potential source of bugs in\n    -    case any callsite forgot to prepare alternates before we iterate through\n    -    the sources.\n    +    This lazy loading doesn't really add much value: the moment where we\n    +    read any object we _have_ to load the alternates anyway. So given that\n    +    most of our commands would access the object database this optimization\n    +    is not really buying us much in the first place. Quite on the contrary,\n    +    it makes the code harder to understand and is a potential source of bugs\n    +    in case any callsite forgot to prepare alternates before we iterate\n    +    through the sources.\n     \n         Historically though there was a reason why we deferred lazy-loading: it\n         may happen that the repository has \"core.ignoreCase\" configured, and we\n3:  2ca1aa2a37 = 3:  df5d7df91d odb: drop `loaded_alternates` field\n4:  1e97c93bdf ! 4:  50a37ef385 odb: drop `alternates_db` field\n    @@ odb.c: void odb_free(struct object_database *o)\n      \tpthread_mutex_destroy(&o->replace_mutex);\n      \n     @@ odb.c: void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n    - \t * the lifetime of the process.\n    + \t * Reprepare alt odbs, in case the alternates file was modified\n    + \t * during the course of this process. This only _adds_ odbs to\n    + \t * the linked list, so existing odbs will continue to exist for\n    +-\t * the lifetime of the process.\n    ++\t * the lifetime of the process. Consequently, we don't have to\n    ++\t * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here.\n      \t */\n      \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n     -\t\todb_prepare_alternates(o);\n\n---\nbase-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765\nchange-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd\n\n"},{"id":"550384","messageId":"20260812-pks-odb-eagerly-prepare-alternates-v2-1-522b9a5bc1ea@pks.im","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"[PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T09:13:57Z","receivedAt":"2026-08-12T09:14:11Z","isPatch":true,"body":"When registering alternates we deduplicate object database sources by\ntheir path so that the same source won't be added twice. Ever since\ncf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\nthis duplicate check is backed by a map keyed by the source's path,\nusing `fspathhash()` and `fspatheq()` as hash and equality functions,\nrespectively.\n\nThese functions are problematic in this context for two reasons:\n\n  - They implicitly depend on `the_repository` instead of the\n    repository that owns the object database.\n\n  - They derive case-sensitivity from `repo_ignore_case()`, which\n    returns a default value in case the repository's configuration has\n    not been parsed yet. Object database sources may be registered\n    before that is the case, so the answer may flip depending on when a\n    source gets registered.\n\nFix this by making the comparison self-contained in the object\ndatabase. Instead of using `fspathhash()` and `fspatheq()` we resolve\n\"core.ignoreCase\" manually and then use the correct comparison function\nbased on the result. This requires us to migrate to a `struct hashmap`,\nas the khash interface does not give us the ability to change these\nfunctions.\n\nNote that we can unconditionally use `strihash()` to compute entry\nhashes regardless of case sensitivity: a hash function only needs to\nguarantee that equal keys have equal hashes, and a case-insensitive\nhash satisfies this requirement for both case-sensitive and\ncase-insensitive equality.\n\nOverall it's quite debatable whether all of this complexity really is\nworth it, or whether we should just linearly search through all sources\nto find duplicates. But the mentioned commit cares about cases with\nthousands of alternates, and a linear search would of course regress\nperformance quite a bit. This doesn't really feel like a reasonable case\nto care about though, but I don't feel comfortable regressing it anyway.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c        | 63 ++++++++++++++++++++++++++++++++++++++++--------------------\n odb.h        | 15 ++++++++++++++-\n odb/source.h |  7 +++++++\n 3 files changed, 63 insertions(+), 22 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex bd02d8ad54..51da386f22 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -2,11 +2,10 @@\n #include \"abspath.h\"\n #include \"commit-graph.h\"\n #include \"config.h\"\n-#include \"dir.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n+#include \"hashmap.h\"\n #include \"hex.h\"\n-#include \"khash.h\"\n #include \"lockfile.h\"\n #include \"loose.h\"\n #include \"midx.h\"\n@@ -29,8 +28,32 @@\n #include \"trace2.h\"\n #include \"write-or-die.h\"\n \n-KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n-\tstruct odb_source *, 1, fspathhash, fspatheq)\n+static int odb_source_paths_cmp(struct object_database *o,\n+\t\t\t\tconst char *a, const char *b)\n+{\n+\tif (o->source_paths_icase < 0) {\n+\t\tint icase = 0;\n+\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n+\t\to->source_paths_icase = icase;\n+\t}\n+\n+\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n+}\n+\n+static int odb_source_by_path_cmp(const void *cb_data,\n+\t\t\t\t  const struct hashmap_entry *entry,\n+\t\t\t\t  const struct hashmap_entry *entry_or_key,\n+\t\t\t\t  const void *keydata)\n+{\n+\tstruct object_database *o = (struct object_database *)cb_data;\n+\tconst struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry);\n+\tconst char *path = keydata;\n+\n+\tif (!path)\n+\t\tpath = container_of(entry_or_key, const struct odb_source, by_path_entry)->path;\n+\n+\treturn odb_source_paths_cmp(o, source->path, path);\n+}\n \n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern)\n@@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb,\n  */\n static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n-\tint r;\n \tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tstruct hashmap_entry key;\n \tbool usable = false;\n \n \tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n@@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \t * Prevent the common mistake of listing the same\n \t * thing twice, or object directory itself.\n \t */\n-\tif (!o->source_by_path) {\n-\t\tkhiter_t p;\n-\n-\t\to->source_by_path = kh_init_odb_path_map();\n+\tif (!hashmap_get_size(&o->source_by_path)) {\n \t\tassert(!o->sources->next);\n-\t\tp = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r);\n-\t\tassert(r == 1); /* never used */\n-\t\tkh_value(o->source_by_path, p) = o->sources;\n+\t\thashmap_entry_init(&o->sources->by_path_entry,\n+\t\t\t\t   strihash(o->sources->path));\n+\t\thashmap_add(&o->source_by_path, &o->sources->by_path_entry);\n \t}\n \n-\tif (fspatheq(path, normalized_objdir.buf))\n+\tif (!odb_source_paths_cmp(o, path, normalized_objdir.buf))\n \t\tgoto out;\n \n-\tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n+\thashmap_entry_init(&key, strihash(path));\n+\tif (hashmap_get(&o->source_by_path, &key, path))\n \t\tgoto out;\n \n \tusable = true;\n@@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n {\n \tstruct odb_source *alternate = NULL;\n \tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n \n \tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n@@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t*odb->sources_tail = alternate;\n \todb->sources_tail = &(alternate->next);\n \n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n+\thashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path));\n+\tif (hashmap_get(&odb->source_by_path, &alternate->by_path_entry,\n+\t\t\talternate->path))\n \t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n+\thashmap_add(&odb->source_by_path, &alternate->by_path_entry);\n \n \t/* recursively add alternates */\n \todb_source_read_alternates(alternate, &sources);\n@@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->repo = repo;\n \tpthread_mutex_init(&o->replace_mutex, NULL);\n \tstring_list_init_dup(&o->submodule_source_paths);\n+\thashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);\n+\to->source_paths_icase = -1;\n \n \tif (flags & ODB_NEW_HONOR_ENV) {\n \t\tprimary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT));\n@@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o)\n \todb_source_free(o->inmemory_objects);\n \to->inmemory_objects = NULL;\n \n-\tkh_destroy_odb_path_map(o->source_by_path);\n-\to->source_by_path = NULL;\n+\thashmap_clear(&o->source_by_path);\n }\n \n void odb_free(struct object_database *o)\ndiff --git a/odb.h b/odb.h\nindex 8eb4e85d64..71af7450a9 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_H\n #define ODB_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"oidset.h\"\n #include \"oidmap.h\"\n@@ -54,7 +55,19 @@ struct object_database {\n \t */\n \tstruct odb_source *sources;\n \tstruct odb_source **sources_tail;\n-\tstruct kh_odb_path_map *source_by_path;\n+\n+\t/*\n+\t * Map of object database sources, keyed by their respective paths.\n+\t * This map is used to detect the case where the same source is\n+\t * registered multiple times.\n+\t */\n+\tstruct hashmap source_by_path;\n+\n+\t/*\n+\t * Whether source paths shall be compared case-insensitively, as\n+\t * determined by \"core.ignoreCase\".\n+\t */\n+\tint source_paths_icase;\n \n \tint loaded_alternates;\n \ndiff --git a/odb/source.h b/odb/source.h\nindex 4bc037b8d6..82cda8ad75 100644\n--- a/odb/source.h\n+++ b/odb/source.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_SOURCE_H\n #define ODB_SOURCE_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -50,6 +51,12 @@ struct strvec;\n struct odb_source {\n \tstruct odb_source *next;\n \n+\t/*\n+\t * Entry in the object database's map of sources, keyed by this\n+\t * source's path.\n+\t */\n+\tstruct hashmap_entry by_path_entry;\n+\n \t/* Object database that owns this object source. */\n \tstruct object_database *odb;\n \n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550385","messageId":"20260812-pks-odb-eagerly-prepare-alternates-v2-2-522b9a5bc1ea@pks.im","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"[PATCH v2 2/4] odb: eagerly initialize alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T09:13:58Z","receivedAt":"2026-08-12T09:14:15Z","isPatch":true,"body":"When creating the object database we initialize the main object database\nsource, but we don't yet initialize its alternates. Instead, we have\nmany calls to `odb_prepare_alternates()` cluttered around the code base\nwhenever we are about to iterate through the sources.\n\nThis lazy loading doesn't really add much value: the moment where we\nread any object we _have_ to load the alternates anyway. So given that\nmost of our commands would access the object database this optimization\nis not really buying us much in the first place. Quite on the contrary,\nit makes the code harder to understand and is a potential source of bugs\nin case any callsite forgot to prepare alternates before we iterate\nthrough the sources.\n\nHistorically though there was a reason why we deferred lazy-loading: it\nmay happen that the repository has \"core.ignoreCase\" configured, and we\nuse that to deduplicate the list of alternates in case we had the same\nalternate configured multiple times, but with different casing. We used\nto initialize the object database before we had fully configured the\nowning repository though, and consequently we couldn't access that\nconfiguration yet. This has changed in the preceding commit though where\nwe started to parse \"core.ignoreCase\" manually.\n\nEagerly prepare alternates both when creating the object database and\nwhen flushing its caches. Drop the now-unneeded calls to prepare the\nalternates that are scattered across the code base.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fsck.c         |  3 ---\n builtin/pack-objects.c |  3 ---\n commit-graph.c         |  4 ----\n loose.c                |  1 -\n object-name.c          |  1 -\n odb.c                  | 26 ++++----------------------\n odb.h                  |  6 ------\n odb/streaming.c        |  1 -\n pack-bitmap.c          |  2 --\n packfile.c             |  1 -\n packfile.h             |  2 --\n 11 files changed, 4 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex a6c054e45b..892c5661d9 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1069,7 +1069,6 @@ int cmd_fsck(int argc,\n \t\todb_for_each_object(repo->objects, NULL,\n \t\t\t\t    mark_object_for_connectivity, repo, 0);\n \t} else {\n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next)\n \t\t\tfsck_source(repo, source);\n \n@@ -1155,7 +1154,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_commit_graph) {\n \t\tstruct child_process commit_graph_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&commit_graph_verify);\n \t\t\tcommit_graph_verify.git_cmd = 1;\n@@ -1173,7 +1171,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_multi_pack_index) {\n \t\tstruct child_process midx_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&midx_verify);\n \t\t\tmidx_verify.git_cmd = 1;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ec5b6f206..48d37e8e32 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1779,8 +1779,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid,\n \t\t*found_offset = 0;\n \t}\n \n-\todb_prepare_alternates(the_repository->objects);\n-\n \tfor (source = the_repository->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\n@@ -4520,7 +4518,6 @@ static void add_objects_in_unpacked_packs(void)\n \t\t.source_infop = &source_info,\n \t};\n \n-\todb_prepare_alternates(to_pack.repo->objects);\n \tfor (source = to_pack.repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \ndiff --git a/commit-graph.c b/commit-graph.c\nindex 49e8f63930..983c11ce85 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -651,8 +651,6 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb,\n \tcount = st->st_size / (odb->repo->hash_algo->hexsz + 1);\n \tCALLOC_ARRAY(oids, count);\n \n-\todb_prepare_alternates(odb);\n-\n \tfor (i = 0; i < count; i++) {\n \t\tstruct odb_source *source;\n \n@@ -768,7 +766,6 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n \tif (!commit_graph_compatible(r))\n \t\treturn NULL;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tr->objects->commit_graph = read_commit_graph_one(source);\n \t\tif (r->objects->commit_graph)\n@@ -2018,7 +2015,6 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)\n \t\t\t_(\"Finding commits for commit graph among packed objects\"),\n \t\t\tctx->approx_nr_objects);\n \n-\todb_prepare_alternates(ctx->r->objects);\n \tfor (source = ctx->r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\todb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi,\ndiff --git a/loose.c b/loose.c\nindex aa3cb1b4fc..c159d29d2d 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -115,7 +115,6 @@ int repo_read_loose_object_map(struct repository *repo)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(repo->objects);\n \tfor (source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (loose_object_map_load(files->loose) < 0)\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..34a08d76dd 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -280,7 +280,6 @@ static int init_object_disambiguation(struct repository *r,\n \n \tds->len = len;\n \tds->repo = r;\n-\todb_prepare_alternates(r->objects);\n \treturn 0;\n }\n \ndiff --git a/odb.c b/odb.c\nindex 51da386f22..2ae8228dd2 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -237,11 +237,6 @@ void odb_add_to_alternates_file(struct object_database *odb,\n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t\t\t\t\t\tconst char *dir)\n {\n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n \treturn odb_add_alternate_recursively(odb, dir, 0);\n }\n \n@@ -250,12 +245,6 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n {\n \tstruct odb_source *source;\n \n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n-\n \t/*\n \t * Make a new primary odb and link the old primary ODB in as an\n \t * alternate\n@@ -361,7 +350,6 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_\n \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n \tstruct strbuf odb_path_real = STRBUF_INIT;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tstrbuf_realpath(&odb_path_real, source->path, 1);\n \t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n@@ -495,7 +483,6 @@ int odb_for_each_alternate(struct object_database *odb,\n \tstruct odb_source *alternate;\n \tint r = 0;\n \n-\todb_prepare_alternates(odb);\n \tfor (alternate = odb->sources->next; alternate; alternate = alternate->next) {\n \t\tr = cb(alternate, payload);\n \t\tif (r)\n@@ -504,7 +491,7 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n@@ -523,7 +510,6 @@ void odb_prepare_alternates(struct object_database *odb)\n \n int odb_has_alternates(struct object_database *odb)\n {\n-\todb_prepare_alternates(odb);\n \treturn !!odb->sources->next;\n }\n \n@@ -583,8 +569,6 @@ static int do_oid_object_info_extended(struct object_database *odb,\n \tif (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags))\n \t\treturn 0;\n \n-\todb_prepare_alternates(odb);\n-\n \twhile (1) {\n \t\tstruct odb_source *source;\n \n@@ -847,7 +831,6 @@ int odb_freshen_object(struct object_database *odb,\n \t\t       const struct object_id *oid)\n {\n \tstruct odb_source *source;\n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (odb_source_freshen_object(source, oid, NULL))\n \t\t\treturn 1;\n@@ -862,7 +845,6 @@ int odb_for_each_object_ext(struct object_database *odb,\n {\n \tint ret;\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tif (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local)\n \t\t\tcontinue;\n@@ -900,7 +882,6 @@ int odb_count_objects(struct object_database *odb,\n \t\treturn 0;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tunsigned long c;\n \n@@ -980,7 +961,6 @@ int odb_find_abbrev_len(struct object_database *odb,\n \t\tgoto out;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tret = odb_source_find_abbrev_len(source, oid, len, &len);\n \t\tif (ret)\n@@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n+\todb_prepare_alternates(o);\n+\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n \t\to->loaded_alternates = 0;\n+\t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\n \n-\todb_prepare_alternates(o);\n \tfor (source = o->sources; source; source = source->next)\n \t\todb_source_prepare(source, flags);\n \ndiff --git a/odb.h b/odb.h\nindex 71af7450a9..fbafee174b 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -273,12 +273,6 @@ void odb_for_each_alternate_ref(struct object_database *odb,\n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern);\n \n-/*\n- * Prepare alternate object sources for the given database by reading\n- * \"objects/info/alternates\" and opening the respective sources.\n- */\n-void odb_prepare_alternates(struct object_database *odb);\n-\n /*\n  * Check whether the object database has any alternates. The primary object\n  * source does not count as alternate.\ndiff --git a/odb/streaming.c b/odb/streaming.c\nindex 20531e864c..37642768e9 100644\n--- a/odb/streaming.c\n+++ b/odb/streaming.c\n@@ -184,7 +184,6 @@ static int istream_source(struct odb_read_stream **out,\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (!odb_source_read_object_stream(out, source, oid))\n \t\t\treturn 0;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e85bd69ba4..e0fb57d332 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -717,7 +717,6 @@ static int open_bitmap(struct repository *r,\n \n \tassert(!bitmap_git->map);\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \n@@ -3417,7 +3416,6 @@ int verify_bitmap_files(struct repository *r)\n \tstruct packed_git *p;\n \tint res = 0;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\ndiff --git a/packfile.c b/packfile.c\nindex 0eee45055f..d870de90ed 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1938,7 +1938,6 @@ int has_object_pack(struct repository *r, const struct object_id *oid)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0))\ndiff --git a/packfile.h b/packfile.h\nindex e1f77152b5..10de24f477 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -77,8 +77,6 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct\n {\n \tstruct repo_for_each_pack_data data = { 0 };\n \n-\todb_prepare_alternates(repo->objects);\n-\n \tfor (struct odb_source *source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct packfile_list_entry *entry = packfile_store_get_packs(files->packed);\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550386","messageId":"20260812-pks-odb-eagerly-prepare-alternates-v2-3-522b9a5bc1ea@pks.im","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"[PATCH v2 3/4] odb: drop `loaded_alternates` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T09:13:59Z","receivedAt":"2026-08-12T09:14:17Z","isPatch":true,"body":"The `struct object_database::loaded_alternates` field tells us whether\nor not alternates have been loaded already. This field was useful before\nthe preceding commit as we were indeed lazy-loading alternates. But now\nthat we started to eagerly load them we can assume them to be loaded\nafter `odb_new()`, and hence the field does not serve any purpose\nanymore.\n\nRemove it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 9 +--------\n odb.h | 2 --\n 2 files changed, 1 insertion(+), 10 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 2ae8228dd2..2eb37a2f44 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \tint ret = odb_source_write_alternate(odb->sources, dir);\n \tif (ret < 0)\n \t\tdie(NULL);\n-\tif (odb->loaded_alternates)\n-\t\todb_add_alternate_recursively(odb, dir, 0);\n+\todb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n@@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tif (odb->loaded_alternates)\n-\t\treturn;\n-\n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n-\todb->loaded_alternates = 1;\n-\n \tstrvec_clear(&sources);\n }\n \n@@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * the lifetime of the process.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\to->loaded_alternates = 0;\n \t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\ndiff --git a/odb.h b/odb.h\nindex fbafee174b..aefb34213f 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,8 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\tint loaded_alternates;\n-\n \t/*\n \t * A list of alternate object directories loaded from the environment;\n \t * this should not generally need to be accessed directly, but will\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550387","messageId":"20260812-pks-odb-eagerly-prepare-alternates-v2-4-522b9a5bc1ea@pks.im","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"[PATCH v2 4/4] odb: drop `alternates_db` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T09:14:00Z","receivedAt":"2026-08-12T09:14:19Z","isPatch":true,"body":"The `struct object_database::alternates_db` field tracks the value of\nthe \"GIT_ALTERNATE_OBJECT_DIRECTORIES\" environment variable and is\nused in `odb_prepare_alternates()`. It's not necessary to store it as a\nseparate field anymore though, as we stopped lazy-loading alternates.\nConsequently, we can simply pass it to `odb_prepare_alternates()` via\n`odb_new()` now.\n\nDo so and remove the field.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 17 +++++++++--------\n odb.h |  7 -------\n 2 files changed, 9 insertions(+), 15 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 2eb37a2f44..0212eaa998 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -490,12 +490,14 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-static void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb,\n+\t\t\t\t   const char *alternate_db)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tparse_alternates(alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n+\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n@@ -1062,11 +1064,11 @@ struct object_database *odb_new(struct repository *repo,\n \n \to->sources = odb_source_new(o, primary_source, true);\n \to->sources_tail = &o->sources->next;\n-\to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n-\todb_prepare_alternates(o);\n+\todb_prepare_alternates(o, secondary_sources);\n \n+\tfree(secondary_sources);\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1100,8 +1102,6 @@ void odb_free(struct object_database *o)\n \tif (!o)\n \t\treturn;\n \n-\tfree(o->alternate_db);\n-\n \toidmap_clear(&o->replace_map, 1);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n@@ -1123,10 +1123,11 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * Reprepare alt odbs, in case the alternates file was modified\n \t * during the course of this process. This only _adds_ odbs to\n \t * the linked list, so existing odbs will continue to exist for\n-\t * the lifetime of the process.\n+\t * the lifetime of the process. Consequently, we don't have to\n+\t * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\todb_prepare_alternates(o);\n+\t\todb_prepare_alternates(o, NULL);\n \t\to->object_count_valid = 0;\n \t}\n \ndiff --git a/odb.h b/odb.h\nindex aefb34213f..748366a610 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,13 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\t/*\n-\t * A list of alternate object directories loaded from the environment;\n-\t * this should not generally need to be accessed directly, but will\n-\t * populate the \"sources\" list when odb_prepare_alternates() is run.\n-\t */\n-\tchar *alternate_db;\n-\n \t/*\n \t * Objects that should be substituted by other objects\n \t * (see git-replace(1)).\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550412","messageId":"xmqqy0ebxsap.fsf@gitster.g","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 0/4] odb: eagerly load alternates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-12T15:38:38Z","receivedAt":"2026-08-12T15:38:42Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\n> with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\n> on-disk structures pluggable, 2026-08-07) merged into it.\n\nIt is not a clean merge, though.  Please double check the\nsynthesized base when I push the integration results out later\ntoday.  d296c52baa (Merge branch 'ps/odb-make-creation-pluggable'\ninto ps/odb-eagerly-load-alternates, 2026-08-12) will be the merge,\nunless I notice and fix a mismerge in it before I push it out.\n\nThanks.\n\n"},{"id":"550485","messageId":"an2Gsf0LzYRKkQkZ@pks.im","threadId":"66146","inReplyTo":"xmqqy0ebxsap.fsf@gitster.g","subject":"Re: [PATCH v2 0/4] odb: eagerly load alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-13T08:56:17Z","receivedAt":"2026-08-13T08:56:23Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 08:38:38AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > The series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\n> > with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\n> > on-disk structures pluggable, 2026-08-07) merged into it.\n> \n> It is not a clean merge, though.  Please double check the\n> synthesized base when I push the integration results out later\n> today.  d296c52baa (Merge branch 'ps/odb-make-creation-pluggable'\n> into ps/odb-eagerly-load-alternates, 2026-08-12) will be the merge,\n> unless I notice and fix a mismerge in it before I push it out.\n\nHm. I'm probably missing something, but your merge is a bit curious as\nyou merge the dependency into the feature branch instead of making it\nthe base of it. If I do the following:\n\n    # Switch to the master commit.\n    $ git switch --detach 010afd3166\n    # Merge the dependnecy.\n    $ git merge e927cfeb21\n\nThen the only merge conflict I get is in \"odb/source-files.c\". This is a\ntrivial merge conflict though, as it only impacts included headers. And\nthen the rest of this series applies on top of that merge base without\nany further issues.\n\nAm I missing something?\n\nThanks!\n\nPatrick\n"},{"id":"550501","messageId":"CAOLa=ZTsumAT6U8+pJQmNjYL6Rt=JkvTJ0V7KQ7MvLYkThTFYA@mail.gmail.com","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-1-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-13T12:23:39Z","receivedAt":"2026-08-13T12:23:42Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When registering alternates we deduplicate object database sources by\n> their path so that the same source won't be added twice. Ever since\n> cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> this duplicate check is backed by a map keyed by the source's path,\n> using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> respectively.\n>\n> These functions are problematic in this context for two reasons:\n>\n>   - They implicitly depend on `the_repository` instead of the\n>     repository that owns the object database.\n>\n>   - They derive case-sensitivity from `repo_ignore_case()`, which\n>     returns a default value in case the repository's configuration has\n>     not been parsed yet. Object database sources may be registered\n>     before that is the case, so the answer may flip depending on when a\n>     source gets registered.\n>\n> Fix this by making the comparison self-contained in the object\n> database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n> \"core.ignoreCase\" manually and then use the correct comparison function\n> based on the result. This requires us to migrate to a `struct hashmap`,\n> as the khash interface does not give us the ability to change these\n> functions.\n>\n> Note that we can unconditionally use `strihash()` to compute entry\n> hashes regardless of case sensitivity: a hash function only needs to\n> guarantee that equal keys have equal hashes, and a case-insensitive\n> hash satisfies this requirement for both case-sensitive and\n> case-insensitive equality.\n>\n> Overall it's quite debatable whether all of this complexity really is\n> worth it, or whether we should just linearly search through all sources\n> to find duplicates. But the mentioned commit cares about cases with\n> thousands of alternates, and a linear search would of course regress\n> performance quite a bit. This doesn't really feel like a reasonable case\n> to care about though, but I don't feel comfortable regressing it anyway.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c        | 63 ++++++++++++++++++++++++++++++++++++++++--------------------\n>  odb.h        | 15 ++++++++++++++-\n>  odb/source.h |  7 +++++++\n>  3 files changed, 63 insertions(+), 22 deletions(-)\n>\n> diff --git a/odb.c b/odb.c\n> index bd02d8ad54..51da386f22 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -2,11 +2,10 @@\n>  #include \"abspath.h\"\n>  #include \"commit-graph.h\"\n>  #include \"config.h\"\n> -#include \"dir.h\"\n>  #include \"environment.h\"\n>  #include \"gettext.h\"\n> +#include \"hashmap.h\"\n>  #include \"hex.h\"\n> -#include \"khash.h\"\n>  #include \"lockfile.h\"\n>  #include \"loose.h\"\n>  #include \"midx.h\"\n> @@ -29,8 +28,32 @@\n>  #include \"trace2.h\"\n>  #include \"write-or-die.h\"\n>\n> -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n> -\tstruct odb_source *, 1, fspathhash, fspatheq)\n> +static int odb_source_paths_cmp(struct object_database *o,\n> +\t\t\t\tconst char *a, const char *b)\n> +{\n> +\tif (o->source_paths_icase < 0) {\n> +\t\tint icase = 0;\n> +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n> +\t\to->source_paths_icase = icase;\n> +\t}\n> +\n\nNit: couldn't this be simplified to\n\nif (o->source_paths_icase < 0)\n   repo_config_get_bool(o->repo, \"core.ignorecase\", &o->source_paths_icase);\n\n> +\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n> +}\n> +\n> +static int odb_source_by_path_cmp(const void *cb_data,\n> +\t\t\t\t  const struct hashmap_entry *entry,\n> +\t\t\t\t  const struct hashmap_entry *entry_or_key,\n> +\t\t\t\t  const void *keydata)\n> +{\n> +\tstruct object_database *o = (struct object_database *)cb_data;\n> +\tconst struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry);\n> +\tconst char *path = keydata;\n> +\n> +\tif (!path)\n> +\t\tpath = container_of(entry_or_key, const struct odb_source, by_path_entry)->path;\n> +\n> +\treturn odb_source_paths_cmp(o, source->path, path);\n> +}\n>\n>  int odb_mkstemp(struct object_database *odb,\n>  \t\tstruct strbuf *temp_filename, const char *pattern)\n> @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb,\n>   */\n>  static bool odb_is_source_usable(struct object_database *o, const char *path)\n>  {\n> -\tint r;\n>  \tstruct strbuf normalized_objdir = STRBUF_INIT;\n> +\tstruct hashmap_entry key;\n>  \tbool usable = false;\n>\n>  \tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n> @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n>  \t * Prevent the common mistake of listing the same\n>  \t * thing twice, or object directory itself.\n>  \t */\n> -\tif (!o->source_by_path) {\n> -\t\tkhiter_t p;\n> -\n> -\t\to->source_by_path = kh_init_odb_path_map();\n> +\tif (!hashmap_get_size(&o->source_by_path)) {\n>  \t\tassert(!o->sources->next);\n> -\t\tp = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r);\n> -\t\tassert(r == 1); /* never used */\n> -\t\tkh_value(o->source_by_path, p) = o->sources;\n> +\t\thashmap_entry_init(&o->sources->by_path_entry,\n> +\t\t\t\t   strihash(o->sources->path));\n> +\t\thashmap_add(&o->source_by_path, &o->sources->by_path_entry);\n>  \t}\n>\n> -\tif (fspatheq(path, normalized_objdir.buf))\n> +\tif (!odb_source_paths_cmp(o, path, normalized_objdir.buf))\n>  \t\tgoto out;\n>\n> -\tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n> +\thashmap_entry_init(&key, strihash(path));\n> +\tif (hashmap_get(&o->source_by_path, &key, path))\n>  \t\tgoto out;\n>\n>  \tusable = true;\n> @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n>  {\n>  \tstruct odb_source *alternate = NULL;\n>  \tstruct strvec sources = STRVEC_INIT;\n> -\tkhiter_t pos;\n> -\tint ret;\n>\n>  \tif (!odb_is_source_usable(odb, source))\n>  \t\tgoto error;\n> @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n>  \t*odb->sources_tail = alternate;\n>  \todb->sources_tail = &(alternate->next);\n>\n> -\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n> -\tif (!ret)\n> +\thashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path));\n> +\tif (hashmap_get(&odb->source_by_path, &alternate->by_path_entry,\n> +\t\t\talternate->path))\n>  \t\tBUG(\"source must not yet exist\");\n> -\tkh_value(odb->source_by_path, pos) = alternate;\n> +\thashmap_add(&odb->source_by_path, &alternate->by_path_entry);\n>\n>  \t/* recursively add alternates */\n>  \todb_source_read_alternates(alternate, &sources);\n> @@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo,\n>  \to->repo = repo;\n>  \tpthread_mutex_init(&o->replace_mutex, NULL);\n>  \tstring_list_init_dup(&o->submodule_source_paths);\n> +\thashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);\n> +\to->source_paths_icase = -1;\n>\n>  \tif (flags & ODB_NEW_HONOR_ENV) {\n>  \t\tprimary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT));\n> @@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o)\n>  \todb_source_free(o->inmemory_objects);\n>  \to->inmemory_objects = NULL;\n>\n> -\tkh_destroy_odb_path_map(o->source_by_path);\n> -\to->source_by_path = NULL;\n> +\thashmap_clear(&o->source_by_path);\n>  }\n>\n>  void odb_free(struct object_database *o)\n> diff --git a/odb.h b/odb.h\n> index 8eb4e85d64..71af7450a9 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -1,6 +1,7 @@\n>  #ifndef ODB_H\n>  #define ODB_H\n>\n> +#include \"hashmap.h\"\n>  #include \"object.h\"\n>  #include \"oidset.h\"\n>  #include \"oidmap.h\"\n> @@ -54,7 +55,19 @@ struct object_database {\n>  \t */\n>  \tstruct odb_source *sources;\n>  \tstruct odb_source **sources_tail;\n> -\tstruct kh_odb_path_map *source_by_path;\n> +\n> +\t/*\n> +\t * Map of object database sources, keyed by their respective paths.\n> +\t * This map is used to detect the case where the same source is\n> +\t * registered multiple times.\n> +\t */\n> +\tstruct hashmap source_by_path;\n> +\n> +\t/*\n> +\t * Whether source paths shall be compared case-insensitively, as\n> +\t * determined by \"core.ignoreCase\".\n> +\t */\n> +\tint source_paths_icase;\n>\n>  \tint loaded_alternates;\n>\n> diff --git a/odb/source.h b/odb/source.h\n> index 4bc037b8d6..82cda8ad75 100644\n> --- a/odb/source.h\n> +++ b/odb/source.h\n> @@ -1,6 +1,7 @@\n>  #ifndef ODB_SOURCE_H\n>  #define ODB_SOURCE_H\n>\n> +#include \"hashmap.h\"\n>  #include \"object.h\"\n>  #include \"odb.h\"\n>  #include \"odb/transaction.h\"\n> @@ -50,6 +51,12 @@ struct strvec;\n>  struct odb_source {\n>  \tstruct odb_source *next;\n>\n> +\t/*\n> +\t * Entry in the object database's map of sources, keyed by this\n> +\t * source's path.\n> +\t */\n> +\tstruct hashmap_entry by_path_entry;\n> +\n>  \t/* Object database that owns this object source. */\n>  \tstruct object_database *odb;\n>\n>\n> --\n> 2.55.0.679.g6767b8d81c.dirty\n\nApart from the nit, this patch looks good.\n"},{"id":"550502","messageId":"CAOLa=ZR1CHHXYjfuJBC0wGqzCYkKUMr2oBbmeUnzz_vCNkegBw@mail.gmail.com","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-3-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 3/4] odb: drop `loaded_alternates` field","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-13T12:25:58Z","receivedAt":"2026-08-13T12:26:00Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The `struct object_database::loaded_alternates` field tells us whether\n> or not alternates have been loaded already. This field was useful before\n> the preceding commit as we were indeed lazy-loading alternates. But now\n> that we started to eagerly load them we can assume them to be loaded\n> after `odb_new()`, and hence the field does not serve any purpose\n> anymore.\n>\n> Remove it.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c | 9 +--------\n>  odb.h | 2 --\n>  2 files changed, 1 insertion(+), 10 deletions(-)\n>\n> diff --git a/odb.c b/odb.c\n> index 2ae8228dd2..2eb37a2f44 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n>  \tint ret = odb_source_write_alternate(odb->sources, dir);\n>  \tif (ret < 0)\n>  \t\tdie(NULL);\n> -\tif (odb->loaded_alternates)\n> -\t\todb_add_alternate_recursively(odb, dir, 0);\n> +\todb_add_alternate_recursively(odb, dir, 0);\n>  }\n>\n>  struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n> @@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb)\n>  {\n>  \tstruct strvec sources = STRVEC_INIT;\n>\n> -\tif (odb->loaded_alternates)\n> -\t\treturn;\n> -\n>  \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n>  \todb_source_read_alternates(odb->sources, &sources);\n>  \tfor (size_t i = 0; i < sources.nr; i++)\n>  \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n>\n> -\todb->loaded_alternates = 1;\n> -\n>  \tstrvec_clear(&sources);\n>  }\n>\n> @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n>  \t * the lifetime of the process.\n>  \t */\n>  \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n> -\t\to->loaded_alternates = 0;\n>  \t\todb_prepare_alternates(o);\n>  \t\to->object_count_valid = 0;\n>  \t}\n\nI was looking at this exact field in the previous commit and wondering\nif it needs to be removed, spot on. Makes sense.\n\n> diff --git a/odb.h b/odb.h\n> index fbafee174b..aefb34213f 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -69,8 +69,6 @@ struct object_database {\n>  \t */\n>  \tint source_paths_icase;\n>\n> -\tint loaded_alternates;\n> -\n>  \t/*\n>  \t * A list of alternate object directories loaded from the environment;\n>  \t * this should not generally need to be accessed directly, but will\n>\n> --\n> 2.55.0.679.g6767b8d81c.dirty\n\nThe patch looks good.\n"},{"id":"550503","messageId":"CAOLa=ZTLq2xjkC12B=4wPJA9UJ1PWS4_iUAyqgcJ7GYnbM2YrQ@mail.gmail.com","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 0/4] odb: eagerly load alternates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-13T12:28:18Z","receivedAt":"2026-08-13T12:28:20Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> when initializing the object database we only eagerly initialize the\n> primary object database source. If the primary source has alternates,\n> those alternates are only initialized the first time we really access\n> the object database.\n>\n> When introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to\n> support multiple object databases., 2005-05-07), alternates were\n> originally only loaded when a given object wasn't found in the primary\n> object database. This was also reinforced by later optimization, for\n> example in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,\n> 2007-05-26), where we tried to avoid loading alternates in even more\n> cases. But as Git has evolved, we eventually started to eagerly parse\n> alternates all over the codebase, including on every single object\n> lookup, and consequently deferring this operation does not really buy us\n> much anymore.\n>\n> The result of this is that we have calls to `odb_prepare_alternates()`\n> cluttered all over the code base. This is somewhat awkward, and as\n> almost every Git command ends up reading objects at it doesn't even buy\n> us anything.\n>\n> This patch series thus gets rid of the lazy-loading. Besides simplifying\n> the codebase a bit, it also prepares us for moving alternates into the\n> \"files\" backend as discussed in [1].\n>\n> The series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\n> with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\n> on-disk structures pluggable, 2026-08-07) merged into it.\n>\n> Changes in v2:\n>   - Add a missing word to a commit message.\n>   - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES\n>     when re-preparing the object database.\n>   - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im\n>\n\nV2 looks good, I have one nit, but it's not work re-rolling :)\n\n> Thanks!\n>\n> Patrick\n>\n> [1]: <amLgMqkqxR8mKIbT@pks.im>\n>\n> ---\n> Patrick Steinhardt (4):\n>       odb: decouple source path comparisons from `the_repository`\n>       odb: eagerly initialize alternates\n>       odb: drop `loaded_alternates` field\n>       odb: drop `alternates_db` field\n>\n>  builtin/fsck.c         |   3 --\n>  builtin/pack-objects.c |   3 --\n>  commit-graph.c         |   4 --\n>  loose.c                |   1 -\n>  object-name.c          |   1 -\n>  odb.c                  | 109 ++++++++++++++++++++++++-------------------------\n>  odb.h                  |  22 +++++-----\n>  odb/source.h           |   7 ++++\n>  odb/streaming.c        |   1 -\n>  pack-bitmap.c          |   2 -\n>  packfile.c             |   1 -\n>  packfile.h             |   2 -\n>  12 files changed, 70 insertions(+), 86 deletions(-)\n>\n> Range-diff versus v1:\n>\n> 1:  25802adffa = 1:  721907c60d odb: decouple source path comparisons from `the_repository`\n> 2:  1e73b730d8 ! 2:  3b2c23566c odb: eagerly initialize alternates\n>     @@ Commit message\n>          many calls to `odb_prepare_alternates()` cluttered around the code base\n>          whenever we are about to iterate through the sources.\n>\n>     -    This lazy loading doesn't really add much value: the moment where read\n>     -    any object we _have_ to load the alternates anyway. So given that most\n>     -    of our commands would access the object database this optimization is\n>     -    not really buying us much in the first place. Quite on the contrary, it\n>     -    makes the code harder to understand and is a potential source of bugs in\n>     -    case any callsite forgot to prepare alternates before we iterate through\n>     -    the sources.\n>     +    This lazy loading doesn't really add much value: the moment where we\n>     +    read any object we _have_ to load the alternates anyway. So given that\n>     +    most of our commands would access the object database this optimization\n>     +    is not really buying us much in the first place. Quite on the contrary,\n>     +    it makes the code harder to understand and is a potential source of bugs\n>     +    in case any callsite forgot to prepare alternates before we iterate\n>     +    through the sources.\n>\n>          Historically though there was a reason why we deferred lazy-loading: it\n>          may happen that the repository has \"core.ignoreCase\" configured, and we\n> 3:  2ca1aa2a37 = 3:  df5d7df91d odb: drop `loaded_alternates` field\n> 4:  1e97c93bdf ! 4:  50a37ef385 odb: drop `alternates_db` field\n>     @@ odb.c: void odb_free(struct object_database *o)\n>       \tpthread_mutex_destroy(&o->replace_mutex);\n>\n>      @@ odb.c: void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n>     - \t * the lifetime of the process.\n>     + \t * Reprepare alt odbs, in case the alternates file was modified\n>     + \t * during the course of this process. This only _adds_ odbs to\n>     + \t * the linked list, so existing odbs will continue to exist for\n>     +-\t * the lifetime of the process.\n>     ++\t * the lifetime of the process. Consequently, we don't have to\n>     ++\t * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here.\n>       \t */\n>       \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n>      -\t\todb_prepare_alternates(o);\n>\n> ---\n> base-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765\n> change-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd\n"},{"id":"550504","messageId":"an3DzPKAFqygOS65@pks.im","threadId":"66146","inReplyTo":"CAOLa=ZTsumAT6U8+pJQmNjYL6Rt=JkvTJ0V7KQ7MvLYkThTFYA@mail.gmail.com","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-13T13:17:00Z","receivedAt":"2026-08-13T13:17:07Z","isPatch":true,"body":"On Thu, Aug 13, 2026 at 05:23:39AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/odb.c b/odb.c\n> > index bd02d8ad54..51da386f22 100644\n> > --- a/odb.c\n> > +++ b/odb.c\n> > @@ -29,8 +28,32 @@\n> >  #include \"trace2.h\"\n> >  #include \"write-or-die.h\"\n> >\n> > -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n> > -\tstruct odb_source *, 1, fspathhash, fspatheq)\n> > +static int odb_source_paths_cmp(struct object_database *o,\n> > +\t\t\t\tconst char *a, const char *b)\n> > +{\n> > +\tif (o->source_paths_icase < 0) {\n> > +\t\tint icase = 0;\n> > +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n> > +\t\to->source_paths_icase = icase;\n> > +\t}\n> > +\n> \n> Nit: couldn't this be simplified to\n> \n> if (o->source_paths_icase < 0)\n>    repo_config_get_bool(o->repo, \"core.ignorecase\", &o->source_paths_icase);\n\nNot quite, as that wouldn't handle the case where the configuration\nisn't set. So we'd retain it as -1 and do the config lookup every single\ntime.\n\nWe could rewrite like this:\n\n\tif (o->source_paths_icase < 0 &&\n\t    repo_config_get_bool(o->repo, \"core.ignorecase\", &icase))\n\t\to->source_paths_icase = 0;\n\nBut I'd argue that this is harder to read.\n\nPatrick\n"},{"id":"550540","messageId":"an36BA_Nw7eLAKYC@denethor","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 0/4] odb: eagerly load alternates","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-08-13T17:09:50Z","receivedAt":"2026-08-13T17:09:52Z","isPatch":true,"body":"On 26/08/12 11:13AM, Patrick Steinhardt wrote:\n> Changes in v2:\n>   - Add a missing word to a commit message.\n>   - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES\n>     when re-preparing the object database.\n\nPer the range-diff, the changes to the log messages in this version\naddress my previous comments/questions. This version looks good to me.\nThanks.\n\n-Justin\n"},{"id":"550601","messageId":"CAOLa=ZQARq2eoVegh1BsnKrvd9MuraNFJ3htKDxQ5H25WJUs1w@mail.gmail.com","threadId":"66146","inReplyTo":"an3DzPKAFqygOS65@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-14T10:21:54Z","receivedAt":"2026-08-14T10:21:59Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Thu, Aug 13, 2026 at 05:23:39AM -0700, Karthik Nayak wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> > diff --git a/odb.c b/odb.c\n>> > index bd02d8ad54..51da386f22 100644\n>> > --- a/odb.c\n>> > +++ b/odb.c\n>> > @@ -29,8 +28,32 @@\n>> >  #include \"trace2.h\"\n>> >  #include \"write-or-die.h\"\n>> >\n>> > -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n>> > -\tstruct odb_source *, 1, fspathhash, fspatheq)\n>> > +static int odb_source_paths_cmp(struct object_database *o,\n>> > +\t\t\t\tconst char *a, const char *b)\n>> > +{\n>> > +\tif (o->source_paths_icase < 0) {\n>> > +\t\tint icase = 0;\n>> > +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n>> > +\t\to->source_paths_icase = icase;\n>> > +\t}\n>> > +\n>>\n>> Nit: couldn't this be simplified to\n>>\n>> if (o->source_paths_icase < 0)\n>>    repo_config_get_bool(o->repo, \"core.ignorecase\", &o->source_paths_icase);\n>\n> Not quite, as that wouldn't handle the case where the configuration\n> isn't set. So we'd retain it as -1 and do the config lookup every single\n> time.\n>\n> We could rewrite like this:\n>\n> \tif (o->source_paths_icase < 0 &&\n> \t    repo_config_get_bool(o->repo, \"core.ignorecase\", &icase))\n> \t\to->source_paths_icase = 0;\n>\n> But I'd argue that this is harder to read.\n>\n> Patrick\n\nOoh, yes, makes sense. It's better as is :)\n"},{"id":"550627","messageId":"20260814171724.GB2563235@coredump.intra.peff.net","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-1-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-14T17:17:24Z","receivedAt":"2026-08-14T17:17:26Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:\n\n> When registering alternates we deduplicate object database sources by\n> their path so that the same source won't be added twice. Ever since\n> cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> this duplicate check is backed by a map keyed by the source's path,\n> using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> respectively.\n> \n> These functions are problematic in this context for two reasons:\n> \n>   - They implicitly depend on `the_repository` instead of the\n>     repository that owns the object database.\n\nI'm not even sure that using core.ignorecase here is strictly correct.\nIt is a property of the containing repository, and the filesystem in\nwhich it's stored. But there is no guarantee that the alternate\ndirectories are in the same repository, or even the same filesystem!\n\nSo it is really just a best guess proxy for \"this system tends to use or\nnot use case insensitive filesystems[1]\". It can be wrong in both\ndirections (failing to suppress duplicates, and suppressing them when\nthey are not actually duplicates).\n\nI wonder how bad it would be if we just always did case-sensitive\ncomparisons and made it the caller's responsibility to spell things\nconsistently.  I guess some names ultimately come from things like\n\"--reference\" command-line arguments, so that would depend on user\nspelling. But having duplicates at all is kind of unlikely (you can't\nget it from one --reference clone, but rather a complex tree of\ninterwoven repos with shared roots).\n\nHow bad is a duplicate alternate? It's a minor performance issue, I'd\nthink. We would add its packs to the list (though hardly ever look\nthrough them, as the \"first\" copy would satisfy most requests, and the\nunused second copies end up at the back of the MRU list). You'd only pay\nthe extra lookup cost for an object which we fail to find entirely,\nwhich is rare-ish (mostly speculative lookups for fetches).\n\nAnd it would fix the unlikely-but-possible opposite case of suppressing\na non-duplicate. If you have a repo on a case-insensitive filesystem\nwith two alternates on a case-sensitive system that differ only in case,\nwe erroneously suppress one of them, and commands may fail to find\nobjects we should have. Of course that's super unlikely, which is why\nnobody has run into it before.\n\nSo I kind of wonder if we could just do away with considering case\ninsensitivity here at all. We'd err on the side of correctness in the\nambiguous cases, and this code complexity can just go away.\n\nAlternatively, I think we could probably make the check more thorough in\na similar way. Always consider a pair of case-insensitive matches as\npossible duplicates, and then for each possible duplicate use stat() to\ncheck their st_dev and st_ino values. That keeps things cheap for normal\ncases, and we pay only the stat() before de-duping. It's correct and\ndoesn't rely on the repo, though it is a bit more somewhat complicated\ncode.\n\n-Peff\n\n[1] Even on a single filesystem I think case-sensitivity check is not\n    completely sufficient either. We know that filesystems do more\n    complicated one-way transformations than just case folding, like\n    unicode normalization or even removing some funky code points.\n    We'd miss those \"equivalent\" spellings.\n"},{"id":"550628","messageId":"20260814172113.GC2563235@coredump.intra.peff.net","threadId":"66146","inReplyTo":"20260812-pks-odb-eagerly-prepare-alternates-v2-1-522b9a5bc1ea@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-14T17:21:13Z","receivedAt":"2026-08-14T17:21:15Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:\n\n> Fix this by making the comparison self-contained in the object\n> database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n> \"core.ignoreCase\" manually and then use the correct comparison function\n> based on the result. This requires us to migrate to a `struct hashmap`,\n> as the khash interface does not give us the ability to change these\n> functions.\n\nBy the way, this bit about khash confused me. We can provide whatever\nhash and equality functions we want. But I'm guessing maybe the issue is\nthat in the khash function interface khash expects, there's no extra\n\"void *\" parameter you can use to store the bit to tell you whether to\nbe case insensitive or not?\n\n-Peff\n"},{"id":"550638","messageId":"xmqqpkzkmsmo.fsf@gitster.g","threadId":"66146","inReplyTo":"20260814171724.GB2563235@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-14T19:03:43Z","receivedAt":"2026-08-14T19:03:45Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> How bad is a duplicate alternate? It's a minor performance issue, I'd\n> think. We would add its packs to the list (though hardly ever look\n> through them, as the \"first\" copy would satisfy most requests, and the\n> unused second copies end up at the back of the MRU list). You'd only pay\n> the extra lookup cost for an object which we fail to find entirely,\n> which is rare-ish (mostly speculative lookups for fetches).\n\nThere may be a future application to be written to go through list\nof alternates---enumerate all objects that exist in the first one,\nand then remove them as duplicates to other alternates.  Oops, there\nwas a duplicated entry and we ended up removing the objects from the\nfirst one registered under a different spelling.\n\nOops (U+1F60F Smirking Face 😏).\n\n> So I kind of wonder if we could just do away with considering case\n> insensitivity here at all. We'd err on the side of correctness in the\n> ambiguous cases, and this code complexity can just go away.\n\nI like the simplicity.\n\n> Alternatively, I think we could probably make the check more thorough in\n> a similar way. Always consider a pair of case-insensitive matches as\n> possible duplicates, and then for each possible duplicate use stat() to\n> check their st_dev and st_ino values. That keeps things cheap for normal\n> cases, and we pay only the stat() before de-duping. It's correct and\n> doesn't rely on the repo, though it is a bit more somewhat complicated\n> code.\n\nHmph, I prefer not to trust st_dev and st_ino on platforms where\ncase insensitivity can possibly become an issue, though.\n\n> [1] Even on a single filesystem I think case-sensitivity check is not\n>     completely sufficient either. We know that filesystems do more\n>     complicated one-way transformations than just case folding, like\n>     unicode normalization or even removing some funky code points.\n>     We'd miss those \"equivalent\" spellings.\n\nmacOS?\n\n"},{"id":"550647","messageId":"20260814203624.GC2575854@coredump.intra.peff.net","threadId":"66146","inReplyTo":"xmqqpkzkmsmo.fsf@gitster.g","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-14T20:36:24Z","receivedAt":"2026-08-14T20:36:26Z","isPatch":true,"body":"On Fri, Aug 14, 2026 at 12:03:43PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > How bad is a duplicate alternate? It's a minor performance issue, I'd\n> > think. We would add its packs to the list (though hardly ever look\n> > through them, as the \"first\" copy would satisfy most requests, and the\n> > unused second copies end up at the back of the MRU list). You'd only pay\n> > the extra lookup cost for an object which we fail to find entirely,\n> > which is rare-ish (mostly speculative lookups for fetches).\n> \n> There may be a future application to be written to go through list\n> of alternates---enumerate all objects that exist in the first one,\n> and then remove them as duplicates to other alternates.  Oops, there\n> was a duplicated entry and we ended up removing the objects from the\n> first one registered under a different spelling.\n\nYeah, that would be dangerous. You _might_ even be able to trigger that\nnow with an object directory that points to itself as an alternate, and\nthen doing \"git repack -adl\" or similar. I don't recall offhand whether\nwe normalize the names or if we'd be fooled by symlinks. Or for that\nmatter if we are even careful about comparing alternates to the main odb\ndirectory.\n\nI hate to be cavalier about conditions that could cause data loss, but\nat the same time...it kind of feels like you'd have to be _trying_ to\nshoot yourself in the foot to create such a situation.\n\n> > Alternatively, I think we could probably make the check more thorough in\n> > a similar way. Always consider a pair of case-insensitive matches as\n> > possible duplicates, and then for each possible duplicate use stat() to\n> > check their st_dev and st_ino values. That keeps things cheap for normal\n> > cases, and we pay only the stat() before de-duping. It's correct and\n> > doesn't rely on the repo, though it is a bit more somewhat complicated\n> > code.\n> \n> Hmph, I prefer not to trust st_dev and st_ino on platforms where\n> case insensitivity can possibly become an issue, though.\n\nYeah, I would prefer not to go down that road, either. There are a lot\nof complexity and portability headaches. I offered it mostly as a \"you\nprobably _could_ do this super-carefully\" option, but my take is that we\ndon't need to be super-careful.\n\n> > [1] Even on a single filesystem I think case-sensitivity check is not\n> >     completely sufficient either. We know that filesystems do more\n> >     complicated one-way transformations than just case folding, like\n> >     unicode normalization or even removing some funky code points.\n> >     We'd miss those \"equivalent\" spellings.\n> \n> macOS?\n\nNaturally. :)\n\n-Peff\n"},{"id":"550680","messageId":"aoKd8g43J6k0xaKN@pks.im","threadId":"66146","inReplyTo":"20260814172113.GC2563235@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T05:36:50Z","receivedAt":"2026-08-17T05:36:57Z","isPatch":true,"body":"On Fri, Aug 14, 2026 at 01:21:13PM -0400, Jeff King wrote:\n> On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:\n> \n> > Fix this by making the comparison self-contained in the object\n> > database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n> > \"core.ignoreCase\" manually and then use the correct comparison function\n> > based on the result. This requires us to migrate to a `struct hashmap`,\n> > as the khash interface does not give us the ability to change these\n> > functions.\n> \n> By the way, this bit about khash confused me. We can provide whatever\n> hash and equality functions we want. But I'm guessing maybe the issue is\n> that in the khash function interface khash expects, there's no extra\n> \"void *\" parameter you can use to store the bit to tell you whether to\n> be case insensitive or not?\n\nAh, yes, that's in fact what I wanted to convey. I'll clarify that a\nbit.\n\nPatrick\n"},{"id":"550681","messageId":"aoKeeQMps50rjhWi@pks.im","threadId":"66146","inReplyTo":"20260814171724.GB2563235@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T05:39:05Z","receivedAt":"2026-08-17T05:39:12Z","isPatch":true,"body":"On Fri, Aug 14, 2026 at 01:17:24PM -0400, Jeff King wrote:\n> On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:\n> \n> > When registering alternates we deduplicate object database sources by\n> > their path so that the same source won't be added twice. Ever since\n> > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> > this duplicate check is backed by a map keyed by the source's path,\n> > using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> > respectively.\n> > \n> > These functions are problematic in this context for two reasons:\n> > \n> >   - They implicitly depend on `the_repository` instead of the\n> >     repository that owns the object database.\n> \n> I'm not even sure that using core.ignorecase here is strictly correct.\n> It is a property of the containing repository, and the filesystem in\n> which it's stored. But there is no guarantee that the alternate\n> directories are in the same repository, or even the same filesystem!\n> \n> So it is really just a best guess proxy for \"this system tends to use or\n> not use case insensitive filesystems[1]\". It can be wrong in both\n> directions (failing to suppress duplicates, and suppressing them when\n> they are not actually duplicates).\n> \n> I wonder how bad it would be if we just always did case-sensitive\n> comparisons and made it the caller's responsibility to spell things\n> consistently.  I guess some names ultimately come from things like\n> \"--reference\" command-line arguments, so that would depend on user\n> spelling. But having duplicates at all is kind of unlikely (you can't\n> get it from one --reference clone, but rather a complex tree of\n> interwoven repos with shared roots).\n> \n> How bad is a duplicate alternate? It's a minor performance issue, I'd\n> think. We would add its packs to the list (though hardly ever look\n> through them, as the \"first\" copy would satisfy most requests, and the\n> unused second copies end up at the back of the MRU list). You'd only pay\n> the extra lookup cost for an object which we fail to find entirely,\n> which is rare-ish (mostly speculative lookups for fetches).\n\nA performance regression is definitely the most likely change in\nbehaviour we might see because of this. One other part I am a bit\nworried about is housekeeping, but I think we should be fine there as we\nonly consider the primary source as special.\n\nI also had the feeling that case insensitivity is quite a bit lacking,\ntoo. What we're really after is whether two directories are actually the\nexact same path. And whether the path is case-insensitive is only one\npart of that equation, so it's an imperfect metric by itself already.\n\nIdeally, we should probably use realpath(3p) to at least also resolve\nsymlinks. Unfortunately, it's not guaranteed that this function also\nknows to canonicalize casing.\n\n> And it would fix the unlikely-but-possible opposite case of suppressing\n> a non-duplicate. If you have a repo on a case-insensitive filesystem\n> with two alternates on a case-sensitive system that differ only in case,\n> we erroneously suppress one of them, and commands may fail to find\n> objects we should have. Of course that's super unlikely, which is why\n> nobody has run into it before.\n> \n> So I kind of wonder if we could just do away with considering case\n> insensitivity here at all. We'd err on the side of correctness in the\n> ambiguous cases, and this code complexity can just go away.\n\nYou will of course be able to craft edge cases where that would be a\nsignificant regression. But if your alternates file looks like this you\nmay be holding it wrong:\n\n    /path/to/alternate\n    /PATH/TO/ALTERNATE\n    /pAtH/tO/aLtErNaTe\n    /PaTh/To/AlTeRnAtE\n\n> Alternatively, I think we could probably make the check more thorough in\n> a similar way. Always consider a pair of case-insensitive matches as\n> possible duplicates, and then for each possible duplicate use stat() to\n> check their st_dev and st_ino values. That keeps things cheap for normal\n> cases, and we pay only the stat() before de-duping. It's correct and\n> doesn't rely on the repo, though it is a bit more somewhat complicated\n> code.\n\nHm. Weren't there filesystems where `st_ino` and `st_dev` aren't set at\nall? I think that's the case on Windows, which is unfortunately also the\none where we see case insensitive filesystems by default. So that makes\nit way less effective, as it only works on systems where we typically\naren't case-insensitive in the first place (except macOS maybe).\n\nSo if we want to go down this path I'm inclined to just unconditionally\nuse case sensitive matching and not introduce any secondary machinery.\n\nPatrick\n"},{"id":"550689","messageId":"aoK1ZYfqh5PnNin6@pks.im","threadId":"66146","inReplyTo":"aoKeeQMps50rjhWi@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T07:16:53Z","receivedAt":"2026-08-17T07:17:00Z","isPatch":true,"body":"On Mon, Aug 17, 2026 at 07:39:05AM +0200, Patrick Steinhardt wrote:\n> On Fri, Aug 14, 2026 at 01:17:24PM -0400, Jeff King wrote:\n> > On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:\n> > \n> > > When registering alternates we deduplicate object database sources by\n> > > their path so that the same source won't be added twice. Ever since\n> > > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> > > this duplicate check is backed by a map keyed by the source's path,\n> > > using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> > > respectively.\n> > > \n> > > These functions are problematic in this context for two reasons:\n> > > \n> > >   - They implicitly depend on `the_repository` instead of the\n> > >     repository that owns the object database.\n> > \n> > I'm not even sure that using core.ignorecase here is strictly correct.\n> > It is a property of the containing repository, and the filesystem in\n> > which it's stored. But there is no guarantee that the alternate\n> > directories are in the same repository, or even the same filesystem!\n> > \n> > So it is really just a best guess proxy for \"this system tends to use or\n> > not use case insensitive filesystems[1]\". It can be wrong in both\n> > directions (failing to suppress duplicates, and suppressing them when\n> > they are not actually duplicates).\n> > \n> > I wonder how bad it would be if we just always did case-sensitive\n> > comparisons and made it the caller's responsibility to spell things\n> > consistently.  I guess some names ultimately come from things like\n> > \"--reference\" command-line arguments, so that would depend on user\n> > spelling. But having duplicates at all is kind of unlikely (you can't\n> > get it from one --reference clone, but rather a complex tree of\n> > interwoven repos with shared roots).\n> > \n> > How bad is a duplicate alternate? It's a minor performance issue, I'd\n> > think. We would add its packs to the list (though hardly ever look\n> > through them, as the \"first\" copy would satisfy most requests, and the\n> > unused second copies end up at the back of the MRU list). You'd only pay\n> > the extra lookup cost for an object which we fail to find entirely,\n> > which is rare-ish (mostly speculative lookups for fetches).\n> \n> A performance regression is definitely the most likely change in\n> behaviour we might see because of this. One other part I am a bit\n> worried about is housekeeping, but I think we should be fine there as we\n> only consider the primary source as special.\n> \n> I also had the feeling that case insensitivity is quite a bit lacking,\n> too. What we're really after is whether two directories are actually the\n> exact same path. And whether the path is case-insensitive is only one\n> part of that equation, so it's an imperfect metric by itself already.\n> \n> Ideally, we should probably use realpath(3p) to at least also resolve\n> symlinks. Unfortunately, it's not guaranteed that this function also\n> knows to canonicalize casing.\n> \n> > And it would fix the unlikely-but-possible opposite case of suppressing\n> > a non-duplicate. If you have a repo on a case-insensitive filesystem\n> > with two alternates on a case-sensitive system that differ only in case,\n> > we erroneously suppress one of them, and commands may fail to find\n> > objects we should have. Of course that's super unlikely, which is why\n> > nobody has run into it before.\n> > \n> > So I kind of wonder if we could just do away with considering case\n> > insensitivity here at all. We'd err on the side of correctness in the\n> > ambiguous cases, and this code complexity can just go away.\n> \n> You will of course be able to craft edge cases where that would be a\n> significant regression. But if your alternates file looks like this you\n> may be holding it wrong:\n> \n>     /path/to/alternate\n>     /PATH/TO/ALTERNATE\n>     /pAtH/tO/aLtErNaTe\n>     /PaTh/To/AlTeRnAtE\n> \n> > Alternatively, I think we could probably make the check more thorough in\n> > a similar way. Always consider a pair of case-insensitive matches as\n> > possible duplicates, and then for each possible duplicate use stat() to\n> > check their st_dev and st_ino values. That keeps things cheap for normal\n> > cases, and we pay only the stat() before de-duping. It's correct and\n> > doesn't rely on the repo, though it is a bit more somewhat complicated\n> > code.\n> \n> Hm. Weren't there filesystems where `st_ino` and `st_dev` aren't set at\n> all? I think that's the case on Windows, which is unfortunately also the\n> one where we see case insensitive filesystems by default. So that makes\n> it way less effective, as it only works on systems where we typically\n> aren't case-insensitive in the first place (except macOS maybe).\n> \n> So if we want to go down this path I'm inclined to just unconditionally\n> use case sensitive matching and not introduce any secondary machinery.\n\nThinking about this a bit more: I'd suggest that we leave this out of\nthis patch and instead document this as a NEEDSWORK area for now. I\n_think_ that this proposed refactoring should be generally fine, and I\nquite like the simplification that results from it. But the risk for\nregression is quite a bit higher compared to the origanal patch that\nI've proposed.\n\nPatrick\n"},{"id":"550691","messageId":"20260817072840.GB690018@coredump.intra.peff.net","threadId":"66146","inReplyTo":"aoKeeQMps50rjhWi@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-17T07:28:40Z","receivedAt":"2026-08-17T07:28:42Z","isPatch":true,"body":"On Mon, Aug 17, 2026 at 07:39:05AM +0200, Patrick Steinhardt wrote:\n\n> I also had the feeling that case insensitivity is quite a bit lacking,\n> too. What we're really after is whether two directories are actually the\n> exact same path. And whether the path is case-insensitive is only one\n> part of that equation, so it's an imperfect metric by itself already.\n> \n> Ideally, we should probably use realpath(3p) to at least also resolve\n> symlinks. Unfortunately, it's not guaranteed that this function also\n> knows to canonicalize casing.\n\nYeah, exactly. I don't think we have a completely robust way of doing\nthat check.\n\n> > So I kind of wonder if we could just do away with considering case\n> > insensitivity here at all. We'd err on the side of correctness in the\n> > ambiguous cases, and this code complexity can just go away.\n> \n> You will of course be able to craft edge cases where that would be a\n> significant regression. But if your alternates file looks like this you\n> may be holding it wrong:\n> \n>     /path/to/alternate\n>     /PATH/TO/ALTERNATE\n>     /pAtH/tO/aLtErNaTe\n>     /PaTh/To/AlTeRnAtE\n\nAgreed. The more likely case to me is that repo \"A\" points to \"B\" and\n\"C\", then \"B\" also points to \"c\" (lowercase). Or you can imagine other\ntree structures that converge.\n\nI don't think you could ever get there with standard Git commands,\nthough. We only ever insert a single alternate via \"clone --shared\", so\nthey always form a chain. To get multiple entries I think you'd have to\ncreate the alternates file manually.\n\n  You could also have a chain that forms a loop, but I think you are\n  probably beyond screwed at that point anyway. And also probably\n  impossible to do with \"clone --shared\", as the parent repo must\n  already exist.\n\nSo yeah, I'd be highly surprised if anybody outside of specialized\nalternates-tweaking scripts (like the ones that forges use) would ever\nconstruct a situation where duplicates even mattered, let alone their\ncase. In the case of GitHub's scripts, they were always boring and\none-level anyway (forks point to a shared repo).\n\nIIRC talking to kernel.org folks long ago, they had some kind of tree\nstructure that matched the filesystem (so foo/bar/baz.git borrowed from\nfoo/bar.git, which borrowed from foo.git). I don't know if it was a\nstrict tree, though, or if that system ever even saw production use.\n\n> Hm. Weren't there filesystems where `st_ino` and `st_dev` aren't set at\n> all? I think that's the case on Windows, which is unfortunately also the\n> one where we see case insensitive filesystems by default. So that makes\n> it way less effective, as it only works on systems where we typically\n> aren't case-insensitive in the first place (except macOS maybe).\n> \n> So if we want to go down this path I'm inclined to just unconditionally\n> use case sensitive matching and not introduce any secondary machinery.\n\nYes, that's my preference, too.\n\n-Peff\n"},{"id":"550692","messageId":"20260817073621.GC690018@coredump.intra.peff.net","threadId":"66146","inReplyTo":"aoK1ZYfqh5PnNin6@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-17T07:36:21Z","receivedAt":"2026-08-17T07:36:22Z","isPatch":true,"body":"On Mon, Aug 17, 2026 at 09:16:53AM +0200, Patrick Steinhardt wrote:\n\n> > So if we want to go down this path I'm inclined to just unconditionally\n> > use case sensitive matching and not introduce any secondary machinery.\n> \n> Thinking about this a bit more: I'd suggest that we leave this out of\n> this patch and instead document this as a NEEDSWORK area for now. I\n> _think_ that this proposed refactoring should be generally fine, and I\n> quite like the simplification that results from it. But the risk for\n> regression is quite a bit higher compared to the origanal patch that\n> I've proposed.\n\nOK. The inline lookup of core.ignoreCase feels quite gross to me, but\nit's _probably_ OK.\n\nThere are all kinds of weird timing issues lurking with config lookup,\nthough. In particular you cache the result in o->source_paths_icase. But\nwould we ever load odb source paths before the repo is fully loaded into\nmemory (or in the case of clone, even fully formed on disk)? In that\ncase we'd cache the wrong value forever.\n\nI think we have repo_ignore_case() now, since e6a79c9eb8 (config: use\nrepo_ignore_case() to access core.ignorecase, 2026-06-19). That's in\n'master', so it might be worth building on that instead. And then if\nthere's any cache invalidation to do, it would eventually happen there.\n\n-Peff\n"},{"id":"550697","messageId":"aoLXioIecFZdGe_O@pks.im","threadId":"66146","inReplyTo":"20260817073621.GC690018@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T09:42:34Z","receivedAt":"2026-08-17T09:42:44Z","isPatch":true,"body":"On Mon, Aug 17, 2026 at 03:36:21AM -0400, Jeff King wrote:\n> On Mon, Aug 17, 2026 at 09:16:53AM +0200, Patrick Steinhardt wrote:\n> \n> > > So if we want to go down this path I'm inclined to just unconditionally\n> > > use case sensitive matching and not introduce any secondary machinery.\n> > \n> > Thinking about this a bit more: I'd suggest that we leave this out of\n> > this patch and instead document this as a NEEDSWORK area for now. I\n> > _think_ that this proposed refactoring should be generally fine, and I\n> > quite like the simplification that results from it. But the risk for\n> > regression is quite a bit higher compared to the origanal patch that\n> > I've proposed.\n> \n> OK. The inline lookup of core.ignoreCase feels quite gross to me, but\n> it's _probably_ OK.\n> \n> There are all kinds of weird timing issues lurking with config lookup,\n> though. In particular you cache the result in o->source_paths_icase. But\n> would we ever load odb source paths before the repo is fully loaded into\n> memory (or in the case of clone, even fully formed on disk)? In that\n> case we'd cache the wrong value forever.\n\nGood callout, there's one gotcha here that I was already fixing in a\nsubsequent patch series. Namely, we call `create_object_directory()`\nbefore we set \"core.sharedRepository\" in `init_db()`. But in all the\nother cases we should be fine.\n\nI'll cherry-pick that patch into this series.\n\n> I think we have repo_ignore_case() now, since e6a79c9eb8 (config: use\n> repo_ignore_case() to access core.ignorecase, 2026-06-19). That's in\n> 'master', so it might be worth building on that instead. And then if\n> there's any cache invalidation to do, it would eventually happen there.\n\nWe can't use that one though, as it uses `repo_config_values()`, and\nthat function only works with `the_repository`. So that'd break with\nsubmodule repositories.\n\nPatrick\n"},{"id":"550698","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im","subject":"[PATCH v3 0/5] odb: eagerly load alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:20Z","receivedAt":"2026-08-17T11:09:29Z","isPatch":true,"body":"Hi,\n\nwhen initializing the object database we only eagerly initialize the\nprimary object database source. If the primary source has alternates,\nthose alternates are only initialized the first time we really access\nthe object database.\n\nWhen introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to\nsupport multiple object databases., 2005-05-07), alternates were\noriginally only loaded when a given object wasn't found in the primary\nobject database. This was also reinforced by later optimization, for\nexample in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,\n2007-05-26), where we tried to avoid loading alternates in even more\ncases. But as Git has evolved, we eventually started to eagerly parse\nalternates all over the codebase, including on every single object\nlookup, and consequently deferring this operation does not really buy us\nmuch anymore.\n\nThe result of this is that we have calls to `odb_prepare_alternates()`\ncluttered all over the code base. This is somewhat awkward, and as\nalmost every Git command ends up reading objects at it doesn't even buy\nus anything.\n\nThis patch series thus gets rid of the lazy-loading. Besides simplifying\nthe codebase a bit, it also prepares us for moving alternates into the\n\"files\" backend as discussed in [1].\n\nThe series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\nwith ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\non-disk structures pluggable, 2026-08-07) merged into it.\n\nChanges in v3:\n  - Create object database after we have written the complete repository\n    configuration in `init_db()`.\n  - Document that we might want to drop case-insensitive deduplication\n    of alternates going forward.\n  - Better explain why we have to migrate to `struct hashmap`.\n  - Link to v2: https://patch.msgid.link/20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im\n\nChanges in v2:\n  - Add a missing word to a commit message.\n  - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES\n    when re-preparing the object database.\n  - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <amLgMqkqxR8mKIbT@pks.im>\n\n---\nPatrick Steinhardt (5):\n      setup: create ref and object databases after config is written\n      odb: decouple source path comparisons from `the_repository`\n      odb: eagerly initialize alternates\n      odb: drop `loaded_alternates` field\n      odb: drop `alternates_db` field\n\n builtin/fsck.c         |   3 --\n builtin/pack-objects.c |   3 --\n commit-graph.c         |   4 --\n loose.c                |   1 -\n object-name.c          |   1 -\n odb.c                  | 124 +++++++++++++++++++++++++++----------------------\n odb.h                  |  22 ++++-----\n odb/source.h           |   7 +++\n odb/streaming.c        |   1 -\n pack-bitmap.c          |   2 -\n packfile.c             |   1 -\n packfile.h             |   2 -\n setup.c                |  12 ++---\n 13 files changed, 91 insertions(+), 92 deletions(-)\n\nRange-diff versus v2:\n\n-:  ---------- > 1:  2adb64d17c setup: create ref and object databases after config is written\n1:  6255ac7964 ! 2:  736b8d8eb4 odb: decouple source path comparisons from `the_repository`\n    @@ Commit message\n         database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n         \"core.ignoreCase\" manually and then use the correct comparison function\n         based on the result. This requires us to migrate to a `struct hashmap`,\n    -    as the khash interface does not give us the ability to change these\n    -    functions.\n    +    as the khash interface does not give us the ability to pass an arbitrary\n    +    payload to these functions, and hence we'd have to use global state to\n    +    decide which of those to use.\n     \n         Note that we can unconditionally use `strihash()` to compute entry\n         hashes regardless of case sensitivity: a hash function only needs to\n    @@ Commit message\n         case-insensitive equality.\n     \n         Overall it's quite debatable whether all of this complexity really is\n    -    worth it, or whether we should just linearly search through all sources\n    -    to find duplicates. But the mentioned commit cares about cases with\n    -    thousands of alternates, and a linear search would of course regress\n    -    performance quite a bit. This doesn't really feel like a reasonable case\n    -    to care about though, but I don't feel comfortable regressing it anyway.\n    +    worth it, out of two reasons:\n    +\n    +      - We could linearly search through all sources to find duplicates. But\n    +        the mentioned commit cares about cases with thousands of alternates,\n    +        and a linear search would of course regress performance quite a bit.\n    +        This doesn't really feel like a reasonable case to care about, but I\n    +        don't feel comfortable regressing it anyway.\n    +\n    +      - It's dubious whether we should handle \"core.ignoreCase\" in the first\n    +        place. The downside would be that we might add the same alternate\n    +        multiple times with different casing. But this is an edge case, and\n    +        it's not even fully fixed because we don't resolve symlinks or\n    +        mountpoints, either.\n    +\n    +    So for now, keep this infrastructure in-place while removing the global\n    +    dependency on `the_repository`. We may want to revisit this in the\n    +    future though.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ odb.c\n      \n     -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n     -\tstruct odb_source *, 1, fspathhash, fspatheq)\n    ++/*\n    ++ * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n    ++ * _may_ be the same. This requires quite a bit of boilerplate for dubious\n    ++ * benefit:\n    ++ *\n    ++ *   - Duplicating alternates should really only lead to regressed performance.\n    ++ *\n    ++ *   - We don't properly resolve symlinks or mointpoints, so we may still end\n    ++ *     up duplicating alternates.\n    ++ *\n    ++ *   - The value may be lying, in which case we might deduplicate alternates\n    ++ *     that are in fact not mapping to the same directory.\n    ++ *\n    ++ * We should investigate whether we can remove this whole mechanism outright.\n    ++ */\n     +static int odb_source_paths_cmp(struct object_database *o,\n     +\t\t\t\tconst char *a, const char *b)\n     +{\n2:  4743659d76 = 3:  a9db918b49 odb: eagerly initialize alternates\n3:  4a62dde9d0 = 4:  369a566a6a odb: drop `loaded_alternates` field\n4:  e978a5a47d = 5:  a0a22a0bd2 odb: drop `alternates_db` field\n\n---\nbase-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765\nchange-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd\n\n"},{"id":"550699","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-1-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"[PATCH v3 1/5] setup: create ref and object databases after config is written","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:21Z","receivedAt":"2026-08-17T11:09:31Z","isPatch":true,"body":"When creating a new repository we create both the reference and object\ndatabases after we have finalized the repository. This ensures that\nthose subsystems find a fully-configured repository at the time where\nthey are asked to create their own on-disk data structures.\n\nThere is one exception though: while we have already fully configured\nthe repository at this point, we haven't yet written both\n\"core.sharedRepository\" and \"receive.denyNonFastforwards\". The latter\nconfiguration doesn't really matter to us, but the first one does as the\n\"files\" object database source reads it.\n\nThis doesn't cause any problems right now, but it will in a subsequent\npatch where we will start to read \"core.ignoreCase\" when creating the\nobject database. Move the initialization of both of these data\nstructures towards the end of `init_db()`. The only thing that now comes\nafter is status reporting, but that's it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n setup.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 20d29f31f4..d90654f584 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2880,12 +2880,6 @@ int init_db(struct repository *repo,\n \treinit = create_default_files(repo, template_dir, original_git_dir,\n \t\t\t\t      &repo_fmt, init_shared_repository);\n \n-\tif (!(flags & INIT_DB_SKIP_REFDB))\n-\t\tcreate_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);\n-\tcreate_object_database(repo);\n-\n-\tstartup_info->have_repository = 1;\n-\n \tif (repo_settings_get_shared_repository(repo)) {\n \t\tchar buf[10];\n \t\t/* We do not spell \"group\" and such, so that\n@@ -2907,6 +2901,12 @@ int init_db(struct repository *repo,\n \t\trepo_config_set(repo, \"receive.denyNonFastforwards\", \"true\");\n \t}\n \n+\tif (!(flags & INIT_DB_SKIP_REFDB))\n+\t\tcreate_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);\n+\tcreate_object_database(repo);\n+\n+\tstartup_info->have_repository = 1;\n+\n \tif (!(flags & INIT_DB_QUIET)) {\n \t\tint len = strlen(git_dir);\n \n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550700","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-2-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"[PATCH v3 2/5] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:22Z","receivedAt":"2026-08-17T11:09:33Z","isPatch":true,"body":"When registering alternates we deduplicate object database sources by\ntheir path so that the same source won't be added twice. Ever since\ncf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\nthis duplicate check is backed by a map keyed by the source's path,\nusing `fspathhash()` and `fspatheq()` as hash and equality functions,\nrespectively.\n\nThese functions are problematic in this context for two reasons:\n\n  - They implicitly depend on `the_repository` instead of the\n    repository that owns the object database.\n\n  - They derive case-sensitivity from `repo_ignore_case()`, which\n    returns a default value in case the repository's configuration has\n    not been parsed yet. Object database sources may be registered\n    before that is the case, so the answer may flip depending on when a\n    source gets registered.\n\nFix this by making the comparison self-contained in the object\ndatabase. Instead of using `fspathhash()` and `fspatheq()` we resolve\n\"core.ignoreCase\" manually and then use the correct comparison function\nbased on the result. This requires us to migrate to a `struct hashmap`,\nas the khash interface does not give us the ability to pass an arbitrary\npayload to these functions, and hence we'd have to use global state to\ndecide which of those to use.\n\nNote that we can unconditionally use `strihash()` to compute entry\nhashes regardless of case sensitivity: a hash function only needs to\nguarantee that equal keys have equal hashes, and a case-insensitive\nhash satisfies this requirement for both case-sensitive and\ncase-insensitive equality.\n\nOverall it's quite debatable whether all of this complexity really is\nworth it, out of two reasons:\n\n  - We could linearly search through all sources to find duplicates. But\n    the mentioned commit cares about cases with thousands of alternates,\n    and a linear search would of course regress performance quite a bit.\n    This doesn't really feel like a reasonable case to care about, but I\n    don't feel comfortable regressing it anyway.\n\n  - It's dubious whether we should handle \"core.ignoreCase\" in the first\n    place. The downside would be that we might add the same alternate\n    multiple times with different casing. But this is an edge case, and\n    it's not even fully fixed because we don't resolve symlinks or\n    mountpoints, either.\n\nSo for now, keep this infrastructure in-place while removing the global\ndependency on `the_repository`. We may want to revisit this in the\nfuture though.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c        | 78 ++++++++++++++++++++++++++++++++++++++++++++----------------\n odb.h        | 15 +++++++++++-\n odb/source.h |  7 ++++++\n 3 files changed, 78 insertions(+), 22 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex bd02d8ad54..22f1425ba5 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -2,11 +2,10 @@\n #include \"abspath.h\"\n #include \"commit-graph.h\"\n #include \"config.h\"\n-#include \"dir.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n+#include \"hashmap.h\"\n #include \"hex.h\"\n-#include \"khash.h\"\n #include \"lockfile.h\"\n #include \"loose.h\"\n #include \"midx.h\"\n@@ -29,8 +28,47 @@\n #include \"trace2.h\"\n #include \"write-or-die.h\"\n \n-KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n-\tstruct odb_source *, 1, fspathhash, fspatheq)\n+/*\n+ * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n+ * _may_ be the same. This requires quite a bit of boilerplate for dubious\n+ * benefit:\n+ *\n+ *   - Duplicating alternates should really only lead to regressed performance.\n+ *\n+ *   - We don't properly resolve symlinks or mointpoints, so we may still end\n+ *     up duplicating alternates.\n+ *\n+ *   - The value may be lying, in which case we might deduplicate alternates\n+ *     that are in fact not mapping to the same directory.\n+ *\n+ * We should investigate whether we can remove this whole mechanism outright.\n+ */\n+static int odb_source_paths_cmp(struct object_database *o,\n+\t\t\t\tconst char *a, const char *b)\n+{\n+\tif (o->source_paths_icase < 0) {\n+\t\tint icase = 0;\n+\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n+\t\to->source_paths_icase = icase;\n+\t}\n+\n+\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n+}\n+\n+static int odb_source_by_path_cmp(const void *cb_data,\n+\t\t\t\t  const struct hashmap_entry *entry,\n+\t\t\t\t  const struct hashmap_entry *entry_or_key,\n+\t\t\t\t  const void *keydata)\n+{\n+\tstruct object_database *o = (struct object_database *)cb_data;\n+\tconst struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry);\n+\tconst char *path = keydata;\n+\n+\tif (!path)\n+\t\tpath = container_of(entry_or_key, const struct odb_source, by_path_entry)->path;\n+\n+\treturn odb_source_paths_cmp(o, source->path, path);\n+}\n \n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern)\n@@ -58,8 +96,8 @@ int odb_mkstemp(struct object_database *odb,\n  */\n static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n-\tint r;\n \tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tstruct hashmap_entry key;\n \tbool usable = false;\n \n \tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n@@ -76,20 +114,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \t * Prevent the common mistake of listing the same\n \t * thing twice, or object directory itself.\n \t */\n-\tif (!o->source_by_path) {\n-\t\tkhiter_t p;\n-\n-\t\to->source_by_path = kh_init_odb_path_map();\n+\tif (!hashmap_get_size(&o->source_by_path)) {\n \t\tassert(!o->sources->next);\n-\t\tp = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r);\n-\t\tassert(r == 1); /* never used */\n-\t\tkh_value(o->source_by_path, p) = o->sources;\n+\t\thashmap_entry_init(&o->sources->by_path_entry,\n+\t\t\t\t   strihash(o->sources->path));\n+\t\thashmap_add(&o->source_by_path, &o->sources->by_path_entry);\n \t}\n \n-\tif (fspatheq(path, normalized_objdir.buf))\n+\tif (!odb_source_paths_cmp(o, path, normalized_objdir.buf))\n \t\tgoto out;\n \n-\tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n+\thashmap_entry_init(&key, strihash(path));\n+\tif (hashmap_get(&o->source_by_path, &key, path))\n \t\tgoto out;\n \n \tusable = true;\n@@ -172,8 +208,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n {\n \tstruct odb_source *alternate = NULL;\n \tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n \n \tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n@@ -184,10 +218,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t*odb->sources_tail = alternate;\n \todb->sources_tail = &(alternate->next);\n \n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n+\thashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path));\n+\tif (hashmap_get(&odb->source_by_path, &alternate->by_path_entry,\n+\t\t\talternate->path))\n \t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n+\thashmap_add(&odb->source_by_path, &alternate->by_path_entry);\n \n \t/* recursively add alternates */\n \todb_source_read_alternates(alternate, &sources);\n@@ -1056,6 +1091,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->repo = repo;\n \tpthread_mutex_init(&o->replace_mutex, NULL);\n \tstring_list_init_dup(&o->submodule_source_paths);\n+\thashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0);\n+\to->source_paths_icase = -1;\n \n \tif (flags & ODB_NEW_HONOR_ENV) {\n \t\tprimary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT));\n@@ -1094,8 +1131,7 @@ static void odb_free_sources(struct object_database *o)\n \todb_source_free(o->inmemory_objects);\n \to->inmemory_objects = NULL;\n \n-\tkh_destroy_odb_path_map(o->source_by_path);\n-\to->source_by_path = NULL;\n+\thashmap_clear(&o->source_by_path);\n }\n \n void odb_free(struct object_database *o)\ndiff --git a/odb.h b/odb.h\nindex 8eb4e85d64..71af7450a9 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_H\n #define ODB_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"oidset.h\"\n #include \"oidmap.h\"\n@@ -54,7 +55,19 @@ struct object_database {\n \t */\n \tstruct odb_source *sources;\n \tstruct odb_source **sources_tail;\n-\tstruct kh_odb_path_map *source_by_path;\n+\n+\t/*\n+\t * Map of object database sources, keyed by their respective paths.\n+\t * This map is used to detect the case where the same source is\n+\t * registered multiple times.\n+\t */\n+\tstruct hashmap source_by_path;\n+\n+\t/*\n+\t * Whether source paths shall be compared case-insensitively, as\n+\t * determined by \"core.ignoreCase\".\n+\t */\n+\tint source_paths_icase;\n \n \tint loaded_alternates;\n \ndiff --git a/odb/source.h b/odb/source.h\nindex 4bc037b8d6..82cda8ad75 100644\n--- a/odb/source.h\n+++ b/odb/source.h\n@@ -1,6 +1,7 @@\n #ifndef ODB_SOURCE_H\n #define ODB_SOURCE_H\n \n+#include \"hashmap.h\"\n #include \"object.h\"\n #include \"odb.h\"\n #include \"odb/transaction.h\"\n@@ -50,6 +51,12 @@ struct strvec;\n struct odb_source {\n \tstruct odb_source *next;\n \n+\t/*\n+\t * Entry in the object database's map of sources, keyed by this\n+\t * source's path.\n+\t */\n+\tstruct hashmap_entry by_path_entry;\n+\n \t/* Object database that owns this object source. */\n \tstruct object_database *odb;\n \n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550701","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-3-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"[PATCH v3 3/5] odb: eagerly initialize alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:23Z","receivedAt":"2026-08-17T11:09:35Z","isPatch":true,"body":"When creating the object database we initialize the main object database\nsource, but we don't yet initialize its alternates. Instead, we have\nmany calls to `odb_prepare_alternates()` cluttered around the code base\nwhenever we are about to iterate through the sources.\n\nThis lazy loading doesn't really add much value: the moment where we\nread any object we _have_ to load the alternates anyway. So given that\nmost of our commands would access the object database this optimization\nis not really buying us much in the first place. Quite on the contrary,\nit makes the code harder to understand and is a potential source of bugs\nin case any callsite forgot to prepare alternates before we iterate\nthrough the sources.\n\nHistorically though there was a reason why we deferred lazy-loading: it\nmay happen that the repository has \"core.ignoreCase\" configured, and we\nuse that to deduplicate the list of alternates in case we had the same\nalternate configured multiple times, but with different casing. We used\nto initialize the object database before we had fully configured the\nowning repository though, and consequently we couldn't access that\nconfiguration yet. This has changed in the preceding commit though where\nwe started to parse \"core.ignoreCase\" manually.\n\nEagerly prepare alternates both when creating the object database and\nwhen flushing its caches. Drop the now-unneeded calls to prepare the\nalternates that are scattered across the code base.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fsck.c         |  3 ---\n builtin/pack-objects.c |  3 ---\n commit-graph.c         |  4 ----\n loose.c                |  1 -\n object-name.c          |  1 -\n odb.c                  | 26 ++++----------------------\n odb.h                  |  6 ------\n odb/streaming.c        |  1 -\n pack-bitmap.c          |  2 --\n packfile.c             |  1 -\n packfile.h             |  2 --\n 11 files changed, 4 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex a6c054e45b..892c5661d9 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -1069,7 +1069,6 @@ int cmd_fsck(int argc,\n \t\todb_for_each_object(repo->objects, NULL,\n \t\t\t\t    mark_object_for_connectivity, repo, 0);\n \t} else {\n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next)\n \t\t\tfsck_source(repo, source);\n \n@@ -1155,7 +1154,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_commit_graph) {\n \t\tstruct child_process commit_graph_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&commit_graph_verify);\n \t\t\tcommit_graph_verify.git_cmd = 1;\n@@ -1173,7 +1171,6 @@ int cmd_fsck(int argc,\n \tif (repo->settings.core_multi_pack_index) {\n \t\tstruct child_process midx_verify = CHILD_PROCESS_INIT;\n \n-\t\todb_prepare_alternates(repo->objects);\n \t\tfor (source = repo->objects->sources; source; source = source->next) {\n \t\t\tchild_process_init(&midx_verify);\n \t\t\tmidx_verify.git_cmd = 1;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ec5b6f206..48d37e8e32 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1779,8 +1779,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid,\n \t\t*found_offset = 0;\n \t}\n \n-\todb_prepare_alternates(the_repository->objects);\n-\n \tfor (source = the_repository->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\n@@ -4520,7 +4518,6 @@ static void add_objects_in_unpacked_packs(void)\n \t\t.source_infop = &source_info,\n \t};\n \n-\todb_prepare_alternates(to_pack.repo->objects);\n \tfor (source = to_pack.repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \ndiff --git a/commit-graph.c b/commit-graph.c\nindex 49e8f63930..983c11ce85 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -651,8 +651,6 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb,\n \tcount = st->st_size / (odb->repo->hash_algo->hexsz + 1);\n \tCALLOC_ARRAY(oids, count);\n \n-\todb_prepare_alternates(odb);\n-\n \tfor (i = 0; i < count; i++) {\n \t\tstruct odb_source *source;\n \n@@ -768,7 +766,6 @@ static struct commit_graph *prepare_commit_graph(struct repository *r)\n \tif (!commit_graph_compatible(r))\n \t\treturn NULL;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tr->objects->commit_graph = read_commit_graph_one(source);\n \t\tif (r->objects->commit_graph)\n@@ -2018,7 +2015,6 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)\n \t\t\t_(\"Finding commits for commit graph among packed objects\"),\n \t\t\tctx->approx_nr_objects);\n \n-\todb_prepare_alternates(ctx->r->objects);\n \tfor (source = ctx->r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\todb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi,\ndiff --git a/loose.c b/loose.c\nindex aa3cb1b4fc..c159d29d2d 100644\n--- a/loose.c\n+++ b/loose.c\n@@ -115,7 +115,6 @@ int repo_read_loose_object_map(struct repository *repo)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(repo->objects);\n \tfor (source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (loose_object_map_load(files->loose) < 0)\ndiff --git a/object-name.c b/object-name.c\nindex 83efba0ba6..34a08d76dd 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -280,7 +280,6 @@ static int init_object_disambiguation(struct repository *r,\n \n \tds->len = len;\n \tds->repo = r;\n-\todb_prepare_alternates(r->objects);\n \treturn 0;\n }\n \ndiff --git a/odb.c b/odb.c\nindex 22f1425ba5..d4917c3678 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -252,11 +252,6 @@ void odb_add_to_alternates_file(struct object_database *odb,\n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t\t\t\t\t\tconst char *dir)\n {\n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n \treturn odb_add_alternate_recursively(odb, dir, 0);\n }\n \n@@ -265,12 +260,6 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n {\n \tstruct odb_source *source;\n \n-\t/*\n-\t * Make sure alternates are initialized, or else our entry may be\n-\t * overwritten when they are.\n-\t */\n-\todb_prepare_alternates(odb);\n-\n \t/*\n \t * Make a new primary odb and link the old primary ODB in as an\n \t * alternate\n@@ -376,7 +365,6 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_\n \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n \tstruct strbuf odb_path_real = STRBUF_INIT;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tstrbuf_realpath(&odb_path_real, source->path, 1);\n \t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n@@ -510,7 +498,6 @@ int odb_for_each_alternate(struct object_database *odb,\n \tstruct odb_source *alternate;\n \tint r = 0;\n \n-\todb_prepare_alternates(odb);\n \tfor (alternate = odb->sources->next; alternate; alternate = alternate->next) {\n \t\tr = cb(alternate, payload);\n \t\tif (r)\n@@ -519,7 +506,7 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n@@ -538,7 +525,6 @@ void odb_prepare_alternates(struct object_database *odb)\n \n int odb_has_alternates(struct object_database *odb)\n {\n-\todb_prepare_alternates(odb);\n \treturn !!odb->sources->next;\n }\n \n@@ -598,8 +584,6 @@ static int do_oid_object_info_extended(struct object_database *odb,\n \tif (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags))\n \t\treturn 0;\n \n-\todb_prepare_alternates(odb);\n-\n \twhile (1) {\n \t\tstruct odb_source *source;\n \n@@ -862,7 +846,6 @@ int odb_freshen_object(struct object_database *odb,\n \t\t       const struct object_id *oid)\n {\n \tstruct odb_source *source;\n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (odb_source_freshen_object(source, oid, NULL))\n \t\t\treturn 1;\n@@ -877,7 +860,6 @@ int odb_for_each_object_ext(struct object_database *odb,\n {\n \tint ret;\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tif (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local)\n \t\t\tcontinue;\n@@ -915,7 +897,6 @@ int odb_count_objects(struct object_database *odb,\n \t\treturn 0;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next) {\n \t\tunsigned long c;\n \n@@ -995,7 +976,6 @@ int odb_find_abbrev_len(struct object_database *odb,\n \t\tgoto out;\n \t}\n \n-\todb_prepare_alternates(odb);\n \tfor (struct odb_source *source = odb->sources; source; source = source->next) {\n \t\tret = odb_source_find_abbrev_len(source, oid, len, &len);\n \t\tif (ret)\n@@ -1106,6 +1086,8 @@ struct object_database *odb_new(struct repository *repo,\n \to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n+\todb_prepare_alternates(o);\n+\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1166,10 +1148,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n \t\to->loaded_alternates = 0;\n+\t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\n \n-\todb_prepare_alternates(o);\n \tfor (source = o->sources; source; source = source->next)\n \t\todb_source_prepare(source, flags);\n \ndiff --git a/odb.h b/odb.h\nindex 71af7450a9..fbafee174b 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -273,12 +273,6 @@ void odb_for_each_alternate_ref(struct object_database *odb,\n int odb_mkstemp(struct object_database *odb,\n \t\tstruct strbuf *temp_filename, const char *pattern);\n \n-/*\n- * Prepare alternate object sources for the given database by reading\n- * \"objects/info/alternates\" and opening the respective sources.\n- */\n-void odb_prepare_alternates(struct object_database *odb);\n-\n /*\n  * Check whether the object database has any alternates. The primary object\n  * source does not count as alternate.\ndiff --git a/odb/streaming.c b/odb/streaming.c\nindex 20531e864c..37642768e9 100644\n--- a/odb/streaming.c\n+++ b/odb/streaming.c\n@@ -184,7 +184,6 @@ static int istream_source(struct odb_read_stream **out,\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(odb);\n \tfor (source = odb->sources; source; source = source->next)\n \t\tif (!odb_source_read_object_stream(out, source, oid))\n \t\t\treturn 0;\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e85bd69ba4..e0fb57d332 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -717,7 +717,6 @@ static int open_bitmap(struct repository *r,\n \n \tassert(!bitmap_git->map);\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \n@@ -3417,7 +3416,6 @@ int verify_bitmap_files(struct repository *r)\n \tstruct packed_git *p;\n \tint res = 0;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct multi_pack_index *m = get_multi_pack_index(files->packed);\ndiff --git a/packfile.c b/packfile.c\nindex 0eee45055f..d870de90ed 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1938,7 +1938,6 @@ int has_object_pack(struct repository *r, const struct object_id *oid)\n {\n \tstruct odb_source *source;\n \n-\todb_prepare_alternates(r->objects);\n \tfor (source = r->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tif (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0))\ndiff --git a/packfile.h b/packfile.h\nindex e1f77152b5..10de24f477 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -77,8 +77,6 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct\n {\n \tstruct repo_for_each_pack_data data = { 0 };\n \n-\todb_prepare_alternates(repo->objects);\n-\n \tfor (struct odb_source *source = repo->objects->sources; source; source = source->next) {\n \t\tstruct odb_source_files *files = odb_source_files_downcast(source);\n \t\tstruct packfile_list_entry *entry = packfile_store_get_packs(files->packed);\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550702","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-4-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"[PATCH v3 4/5] odb: drop `loaded_alternates` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:24Z","receivedAt":"2026-08-17T11:09:37Z","isPatch":true,"body":"The `struct object_database::loaded_alternates` field tells us whether\nor not alternates have been loaded already. This field was useful before\nthe preceding commit as we were indeed lazy-loading alternates. But now\nthat we started to eagerly load them we can assume them to be loaded\nafter `odb_new()`, and hence the field does not serve any purpose\nanymore.\n\nRemove it.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 9 +--------\n odb.h | 2 --\n 2 files changed, 1 insertion(+), 10 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex d4917c3678..ada42f864b 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -245,8 +245,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \tint ret = odb_source_write_alternate(odb->sources, dir);\n \tif (ret < 0)\n \t\tdie(NULL);\n-\tif (odb->loaded_alternates)\n-\t\todb_add_alternate_recursively(odb, dir, 0);\n+\todb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n@@ -510,16 +509,11 @@ static void odb_prepare_alternates(struct object_database *odb)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tif (odb->loaded_alternates)\n-\t\treturn;\n-\n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n-\todb->loaded_alternates = 1;\n-\n \tstrvec_clear(&sources);\n }\n \n@@ -1147,7 +1141,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * the lifetime of the process.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\to->loaded_alternates = 0;\n \t\todb_prepare_alternates(o);\n \t\to->object_count_valid = 0;\n \t}\ndiff --git a/odb.h b/odb.h\nindex fbafee174b..aefb34213f 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,8 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\tint loaded_alternates;\n-\n \t/*\n \t * A list of alternate object directories loaded from the environment;\n \t * this should not generally need to be accessed directly, but will\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550703","messageId":"20260817-pks-odb-eagerly-prepare-alternates-v3-5-1115a7e02467@pks.im","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"[PATCH v3 5/5] odb: drop `alternates_db` field","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-17T11:09:25Z","receivedAt":"2026-08-17T11:09:39Z","isPatch":true,"body":"The `struct object_database::alternates_db` field tracks the value of\nthe \"GIT_ALTERNATE_OBJECT_DIRECTORIES\" environment variable and is\nused in `odb_prepare_alternates()`. It's not necessary to store it as a\nseparate field anymore though, as we stopped lazy-loading alternates.\nConsequently, we can simply pass it to `odb_prepare_alternates()` via\n`odb_new()` now.\n\nDo so and remove the field.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 17 +++++++++--------\n odb.h |  7 -------\n 2 files changed, 9 insertions(+), 15 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex ada42f864b..115957e983 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -505,12 +505,14 @@ int odb_for_each_alternate(struct object_database *odb,\n \treturn r;\n }\n \n-static void odb_prepare_alternates(struct object_database *odb)\n+static void odb_prepare_alternates(struct object_database *odb,\n+\t\t\t\t   const char *alternate_db)\n {\n \tstruct strvec sources = STRVEC_INIT;\n \n-\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tparse_alternates(alternate_db, PATH_SEP, NULL, &sources);\n \todb_source_read_alternates(odb->sources, &sources);\n+\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n@@ -1077,11 +1079,11 @@ struct object_database *odb_new(struct repository *repo,\n \n \to->sources = odb_source_new(o, primary_source, true);\n \to->sources_tail = &o->sources->next;\n-\to->alternate_db = secondary_sources;\n \to->inmemory_objects = &odb_source_inmemory_new(o)->base;\n \n-\todb_prepare_alternates(o);\n+\todb_prepare_alternates(o, secondary_sources);\n \n+\tfree(secondary_sources);\n \tfree(primary_source);\n \treturn o;\n }\n@@ -1115,8 +1117,6 @@ void odb_free(struct object_database *o)\n \tif (!o)\n \t\treturn;\n \n-\tfree(o->alternate_db);\n-\n \toidmap_clear(&o->replace_map, 1);\n \tpthread_mutex_destroy(&o->replace_mutex);\n \n@@ -1138,10 +1138,11 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)\n \t * Reprepare alt odbs, in case the alternates file was modified\n \t * during the course of this process. This only _adds_ odbs to\n \t * the linked list, so existing odbs will continue to exist for\n-\t * the lifetime of the process.\n+\t * the lifetime of the process. Consequently, we don't have to\n+\t * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here.\n \t */\n \tif (flags & ODB_PREPARE_FLUSH_CACHES) {\n-\t\todb_prepare_alternates(o);\n+\t\todb_prepare_alternates(o, NULL);\n \t\to->object_count_valid = 0;\n \t}\n \ndiff --git a/odb.h b/odb.h\nindex aefb34213f..748366a610 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -69,13 +69,6 @@ struct object_database {\n \t */\n \tint source_paths_icase;\n \n-\t/*\n-\t * A list of alternate object directories loaded from the environment;\n-\t * this should not generally need to be accessed directly, but will\n-\t * populate the \"sources\" list when odb_prepare_alternates() is run.\n-\t */\n-\tchar *alternate_db;\n-\n \t/*\n \t * Objects that should be substituted by other objects\n \t * (see git-replace(1)).\n\n-- \n2.55.0.822.g20453c30eb.dirty\n\n"},{"id":"550715","messageId":"20260817174716.GA732563@coredump.intra.peff.net","threadId":"66146","inReplyTo":"aoLXioIecFZdGe_O@pks.im","subject":"Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-17T17:47:16Z","receivedAt":"2026-08-17T17:47:18Z","isPatch":true,"body":"On Mon, Aug 17, 2026 at 11:42:34AM +0200, Patrick Steinhardt wrote:\n\n> > I think we have repo_ignore_case() now, since e6a79c9eb8 (config: use\n> > repo_ignore_case() to access core.ignorecase, 2026-06-19). That's in\n> > 'master', so it might be worth building on that instead. And then if\n> > there's any cache invalidation to do, it would eventually happen there.\n> \n> We can't use that one though, as it uses `repo_config_values()`, and\n> that function only works with `the_repository`. So that'd break with\n> submodule repositories.\n\nOh, wow. I looked at the function and saw that it took a repository\narguments. But then repo_config_values() does a BUG() when you pass in\nanything but the_repository. That's...surprising. And gross.\n\nBut yeah, I agree it's not yet ready for your use here.\n\n-Peff\n"},{"id":"550880","messageId":"CAOLa=ZT-ObRJ3t0XYAkL33CPDZ2ULu_5M7c477rr0pZBqTBi9w@mail.gmail.com","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-1-1115a7e02467@pks.im","subject":"Re: [PATCH v3 1/5] setup: create ref and object databases after config is written","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T09:09:48Z","receivedAt":"2026-08-20T09:09:51Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When creating a new repository we create both the reference and object\n> databases after we have finalized the repository. This ensures that\n> those subsystems find a fully-configured repository at the time where\n> they are asked to create their own on-disk data structures.\n>\n> There is one exception though: while we have already fully configured\n> the repository at this point, we haven't yet written both\n> \"core.sharedRepository\" and \"receive.denyNonFastforwards\". The latter\n> configuration doesn't really matter to us, but the first one does as the\n> \"files\" object database source reads it.\n>\n> This doesn't cause any problems right now, but it will in a subsequent\n> patch where we will start to read \"core.ignoreCase\" when creating the\n> object database. Move the initialization of both of these data\n> structures towards the end of `init_db()`. The only thing that now comes\n> after is status reporting, but that's it.\n>\n\nOkay so this is the new patch in this version. So since we now read\nconfig as part of object database creation, that means we would need to\nknow the value of 'core.sharedRepository' and that can't happen if the\nodb is initialized before that. Alright makes sense.\n\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  setup.c | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n>\n> diff --git a/setup.c b/setup.c\n> index 20d29f31f4..d90654f584 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -2880,12 +2880,6 @@ int init_db(struct repository *repo,\n>  \treinit = create_default_files(repo, template_dir, original_git_dir,\n>  \t\t\t\t      &repo_fmt, init_shared_repository);\n>\n> -\tif (!(flags & INIT_DB_SKIP_REFDB))\n> -\t\tcreate_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);\n> -\tcreate_object_database(repo);\n> -\n> -\tstartup_info->have_repository = 1;\n> -\n>  \tif (repo_settings_get_shared_repository(repo)) {\n>  \t\tchar buf[10];\n>  \t\t/* We do not spell \"group\" and such, so that\n> @@ -2907,6 +2901,12 @@ int init_db(struct repository *repo,\n>  \t\trepo_config_set(repo, \"receive.denyNonFastforwards\", \"true\");\n>  \t}\n>\n> +\tif (!(flags & INIT_DB_SKIP_REFDB))\n> +\t\tcreate_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);\n> +\tcreate_object_database(repo);\n> +\n> +\tstartup_info->have_repository = 1;\n> +\n>  \tif (!(flags & INIT_DB_QUIET)) {\n>  \t\tint len = strlen(git_dir);\n>\n>\n> --\n> 2.55.0.822.g20453c30eb.dirty\n"},{"id":"550881","messageId":"CAOLa=ZReodSXjEbQkFoxcofMLq6mUOjXANRg7bZ2uEKKQn=DXw@mail.gmail.com","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im","subject":"Re: [PATCH v3 0/5] odb: eagerly load alternates","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-20T09:11:07Z","receivedAt":"2026-08-20T09:11:10Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> when initializing the object database we only eagerly initialize the\n> primary object database source. If the primary source has alternates,\n> those alternates are only initialized the first time we really access\n> the object database.\n>\n> When introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to\n> support multiple object databases., 2005-05-07), alternates were\n> originally only loaded when a given object wasn't found in the primary\n> object database. This was also reinforced by later optimization, for\n> example in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,\n> 2007-05-26), where we tried to avoid loading alternates in even more\n> cases. But as Git has evolved, we eventually started to eagerly parse\n> alternates all over the codebase, including on every single object\n> lookup, and consequently deferring this operation does not really buy us\n> much anymore.\n>\n> The result of this is that we have calls to `odb_prepare_alternates()`\n> cluttered all over the code base. This is somewhat awkward, and as\n> almost every Git command ends up reading objects at it doesn't even buy\n> us anything.\n>\n> This patch series thus gets rid of the lazy-loading. Besides simplifying\n> the codebase a bit, it also prepares us for moving alternates into the\n> \"files\" backend as discussed in [1].\n>\n> The series is built on top of 010afd3166 (The 12th batch, 2026-08-07)\n> with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of\n> on-disk structures pluggable, 2026-08-07) merged into it.\n>\n> Changes in v3:\n>   - Create object database after we have written the complete repository\n>     configuration in `init_db()`.\n>   - Document that we might want to drop case-insensitive deduplication\n>     of alternates going forward.\n>   - Better explain why we have to migrate to `struct hashmap`.\n>   - Link to v2: https://patch.msgid.link/20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im\n>\n> Changes in v2:\n>   - Add a missing word to a commit message.\n>   - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES\n>     when re-preparing the object database.\n>   - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im\n>\n> Thanks!\n>\n> Patrick\n>\n> [1]: <amLgMqkqxR8mKIbT@pks.im>\n>\n> ---\n> Patrick Steinhardt (5):\n>       setup: create ref and object databases after config is written\n>       odb: decouple source path comparisons from `the_repository`\n>       odb: eagerly initialize alternates\n>       odb: drop `loaded_alternates` field\n>       odb: drop `alternates_db` field\n>\n>  builtin/fsck.c         |   3 --\n>  builtin/pack-objects.c |   3 --\n>  commit-graph.c         |   4 --\n>  loose.c                |   1 -\n>  object-name.c          |   1 -\n>  odb.c                  | 124 +++++++++++++++++++++++++++----------------------\n>  odb.h                  |  22 ++++-----\n>  odb/source.h           |   7 +++\n>  odb/streaming.c        |   1 -\n>  pack-bitmap.c          |   2 -\n>  packfile.c             |   1 -\n>  packfile.h             |   2 -\n>  setup.c                |  12 ++---\n>  13 files changed, 91 insertions(+), 92 deletions(-)\n>\n> Range-diff versus v2:\n>\n> -:  ---------- > 1:  2adb64d17c setup: create ref and object databases after config is written\n> 1:  6255ac7964 ! 2:  736b8d8eb4 odb: decouple source path comparisons from `the_repository`\n>     @@ Commit message\n>          database. Instead of using `fspathhash()` and `fspatheq()` we resolve\n>          \"core.ignoreCase\" manually and then use the correct comparison function\n>          based on the result. This requires us to migrate to a `struct hashmap`,\n>     -    as the khash interface does not give us the ability to change these\n>     -    functions.\n>     +    as the khash interface does not give us the ability to pass an arbitrary\n>     +    payload to these functions, and hence we'd have to use global state to\n>     +    decide which of those to use.\n>\n>          Note that we can unconditionally use `strihash()` to compute entry\n>          hashes regardless of case sensitivity: a hash function only needs to\n>     @@ Commit message\n>          case-insensitive equality.\n>\n>          Overall it's quite debatable whether all of this complexity really is\n>     -    worth it, or whether we should just linearly search through all sources\n>     -    to find duplicates. But the mentioned commit cares about cases with\n>     -    thousands of alternates, and a linear search would of course regress\n>     -    performance quite a bit. This doesn't really feel like a reasonable case\n>     -    to care about though, but I don't feel comfortable regressing it anyway.\n>     +    worth it, out of two reasons:\n>     +\n>     +      - We could linearly search through all sources to find duplicates. But\n>     +        the mentioned commit cares about cases with thousands of alternates,\n>     +        and a linear search would of course regress performance quite a bit.\n>     +        This doesn't really feel like a reasonable case to care about, but I\n>     +        don't feel comfortable regressing it anyway.\n>     +\n>     +      - It's dubious whether we should handle \"core.ignoreCase\" in the first\n>     +        place. The downside would be that we might add the same alternate\n>     +        multiple times with different casing. But this is an edge case, and\n>     +        it's not even fully fixed because we don't resolve symlinks or\n>     +        mountpoints, either.\n>     +\n>     +    So for now, keep this infrastructure in-place while removing the global\n>     +    dependency on `the_repository`. We may want to revisit this in the\n>     +    future though.\n>\n>          Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>\n>     @@ odb.c\n>\n>      -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,\n>      -\tstruct odb_source *, 1, fspathhash, fspatheq)\n>     ++/*\n>     ++ * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n>     ++ * _may_ be the same. This requires quite a bit of boilerplate for dubious\n>     ++ * benefit:\n>     ++ *\n>     ++ *   - Duplicating alternates should really only lead to regressed performance.\n>     ++ *\n>     ++ *   - We don't properly resolve symlinks or mointpoints, so we may still end\n>     ++ *     up duplicating alternates.\n>     ++ *\n>     ++ *   - The value may be lying, in which case we might deduplicate alternates\n>     ++ *     that are in fact not mapping to the same directory.\n>     ++ *\n>     ++ * We should investigate whether we can remove this whole mechanism outright.\n>     ++ */\n>      +static int odb_source_paths_cmp(struct object_database *o,\n>      +\t\t\t\tconst char *a, const char *b)\n>      +{\n> 2:  4743659d76 = 3:  a9db918b49 odb: eagerly initialize alternates\n> 3:  4a62dde9d0 = 4:  369a566a6a odb: drop `loaded_alternates` field\n> 4:  e978a5a47d = 5:  a0a22a0bd2 odb: drop `alternates_db` field\n>\n\nThe new patch and the range-diff looks good. Thanks!\n"},{"id":"550918","messageId":"xmqqmrugsryl.fsf@gitster.g","threadId":"66146","inReplyTo":"20260817-pks-odb-eagerly-prepare-alternates-v3-2-1115a7e02467@pks.im","subject":"Re: [PATCH v3 2/5] odb: decouple source path comparisons from `the_repository`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-20T15:59:46Z","receivedAt":"2026-08-20T15:59:50Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When registering alternates we deduplicate object database sources by\n> their path so that the same source won't be added twice. Ever since\n> cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> this duplicate check is backed by a map keyed by the source's path,\n> using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> respectively.\n>\n> These functions are problematic in this context for two reasons:\n>\n>   - They implicitly depend on `the_repository` instead of the\n>     repository that owns the object database.\n>\n>   - They derive case-sensitivity from `repo_ignore_case()`, which\n>     returns a default value in case the repository's configuration has\n>     not been parsed yet. Object database sources may be registered\n>     before that is the case, so the answer may flip depending on when a\n>     source gets registered.\n\nAs you later mention, we can always hash case-insensitively with\nthe downside of additional possibilities of hash collisions.  I\nwould not be too worried about the hash side, but the above makes\nme wonder what should happen in the eq() function when a repository\nuses object databases living on separate filesystems, some being\ncase-insensitive and others being case-sensitive.\n\nIn any case, I wonder if 'core.ignoreCase' should even be a part of\nthe repository configuration.  Do we need to support\nconfigurations where some parts of the repository are backed by a\ncase-insensitive filesystem while others are not?  And if so,\nhow?  It almost feels as if each of these object database sources\nneeds to report \"This is the path to my filesystem location, and\nthe path may have case-different aliases\" and \"My path is on a\ncase-sensitive filesystem so you do not have to worry about it\nclashing\", and we need to compare them accordingly.\n\n> Overall it's quite debatable whether all of this complexity really is\n> worth it, out of two reasons:\n>\n>   - We could linearly search through all sources to find duplicates. But\n>     the mentioned commit cares about cases with thousands of alternates,\n>     and a linear search would of course regress performance quite a bit.\n>     This doesn't really feel like a reasonable case to care about, but I\n>     don't feel comfortable regressing it anyway.\n\nLinear or hashed, the issue of what the definition of eq() should be\nremains.  Discarding the hash map does not help at all, I suspect.\nAm I missing something?\n\n>   - It's dubious whether we should handle \"core.ignoreCase\" in the first\n>     place. The downside would be that we might add the same alternate\n>     multiple times with different casing. But this is an edge case, and\n>     it's not even fully fixed because we don't resolve symlinks or\n>     mountpoints, either.\n\nDo we know if these all come directly from the way the user spelled\nthese paths?\n\nUnless there is a demon that randomly flips the character case in a\npathname once it is obtained from the user or readdir() before it\ngets to this code path, an easy way out may be to tell users \"don't\nspell the pathnames inconsistently\" or its equivalent, \"do spell\nthem exactly the way readdir() would report on your system\", with \"if\nyou fail to do so, bad things will happen\".  I suspect that the bad\nthing in this particular case is merely that a search in the\nalternates is made unnecessarily inefficient due to duplicates, so it\nmay be a reasonable alternative.\n\nAlternatively, we can even say \"your repository cannot span\nfilesystems with different case sensitivities\"; I am sure there\nwould be some users affected by such a declaration, but I do not\nknow how much we should care.\n\n> +/*\n> + * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n> + * _may_ be the same. This requires quite a bit of boilerplate for dubious\n> + * benefit:\n> + *\n> + *   - Duplicating alternates should really only lead to regressed performance.\n> + *\n> + *   - We don't properly resolve symlinks or mointpoints, so we may still end\n> + *     up duplicating alternates.\n> + *\n> + *   - The value may be lying, in which case we might deduplicate alternates\n> + *     that are in fact not mapping to the same directory.\n> + *\n> + * We should investigate whether we can remove this whole mechanism outright.\n> + */\n> +static int odb_source_paths_cmp(struct object_database *o,\n> +\t\t\t\tconst char *a, const char *b)\n> +{\n> +\tif (o->source_paths_icase < 0) {\n> +\t\tint icase = 0;\n> +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n\nI suspect accessing o->repo should be safe even in the\ninitialization sequence, simply because \"o->repo = repo\" is done as\nthe first thing in odb_new(), but do we know o->repo->initialized is\ntrue in this code path?  Refraining from making that call and\nassuming a case senstivie comparison may be necessary when o->repo\nis not yet initialized.\n\n> +\t\to->source_paths_icase = icase;\n> +\t}\n> +\n> +\treturn o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b);\n> +}\n"},{"id":"550995","messageId":"aof-t_bRzC0u1hHj@pks.im","threadId":"66146","inReplyTo":"xmqqmrugsryl.fsf@gitster.g","subject":"Re: [PATCH v3 2/5] odb: decouple source path comparisons from `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-21T07:31:03Z","receivedAt":"2026-08-21T07:31:16Z","isPatch":true,"body":"On Thu, Aug 20, 2026 at 08:59:46AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > When registering alternates we deduplicate object database sources by\n> > their path so that the same source won't be added twice. Ever since\n> > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)\n> > this duplicate check is backed by a map keyed by the source's path,\n> > using `fspathhash()` and `fspatheq()` as hash and equality functions,\n> > respectively.\n> >\n> > These functions are problematic in this context for two reasons:\n> >\n> >   - They implicitly depend on `the_repository` instead of the\n> >     repository that owns the object database.\n> >\n> >   - They derive case-sensitivity from `repo_ignore_case()`, which\n> >     returns a default value in case the repository's configuration has\n> >     not been parsed yet. Object database sources may be registered\n> >     before that is the case, so the answer may flip depending on when a\n> >     source gets registered.\n> \n> As you later mention, we can always hash case-insensitively with\n> the downside of additional possibilities of hash collisions.  I\n> would not be too worried about the hash side, but the above makes\n> me wonder what should happen in the eq() function when a repository\n> uses object databases living on separate filesystems, some being\n> case-insensitive and others being case-sensitive.\n> \n> In any case, I wonder if 'core.ignoreCase' should even be a part of\n> the repository configuration.  Do we need to support\n> configurations where some parts of the repository are backed by a\n> case-insensitive filesystem while others are not?  And if so,\n> how?  It almost feels as if each of these object database sources\n> needs to report \"This is the path to my filesystem location, and\n> the path may have case-different aliases\" and \"My path is on a\n> case-sensitive filesystem so you do not have to worry about it\n> clashing\", and we need to compare them accordingly.\n\nIn theory you can of course construct cases where we'd need that. But\nthis whole mechanism is very old already, and I don't know about a\nsingle reported case where it caused problems. So yes, it is imperfect,\nbut I think we're overcomplicating things that have been working just\nfine for the last 20 years in practice.\n\n> > Overall it's quite debatable whether all of this complexity really is\n> > worth it, out of two reasons:\n> >\n> >   - We could linearly search through all sources to find duplicates. But\n> >     the mentioned commit cares about cases with thousands of alternates,\n> >     and a linear search would of course regress performance quite a bit.\n> >     This doesn't really feel like a reasonable case to care about, but I\n> >     don't feel comfortable regressing it anyway.\n> \n> Linear or hashed, the issue of what the definition of eq() should be\n> remains.  Discarding the hash map does not help at all, I suspect.\n> Am I missing something?\n\nNo, you're not missing anything here. This was more of a \"using a\nhashmap in general feels overengineered\" statement, as you'd typically\nonly have at most a handful of them anyway.\n\nDoing a couple of string comparisons would likely be more efficient\ncompared to spinning up the whole hashmap machinery if you really only\nhave one or two alternates, which is going to be 99% of all the use\ncases out there. Having the hashmap probably only starts to make sense\nonce you have a couple dozen or even hundreds of alternates, so I have a\nfeeling that we overoptimized for a mostly theoretical scenario here.\n\nI don't feel comfortable removing that mechanism though. There's always\nthat one person relying on those weird edge cases.\n\n> >   - It's dubious whether we should handle \"core.ignoreCase\" in the first\n> >     place. The downside would be that we might add the same alternate\n> >     multiple times with different casing. But this is an edge case, and\n> >     it's not even fully fixed because we don't resolve symlinks or\n> >     mountpoints, either.\n> \n> Do we know if these all come directly from the way the user spelled\n> these paths?\n\nWe have two different sources, \"objects/info/alternates\" and\n\"GIT_ALTERNATE_OBJECT_DIRECTORIES\", both of which are parsed via\n`parse_alternates()`. That function knows to translate relative paths\ninto absolute ones and it normalizes the result via `realpath()`. So no,\nthey're not exactly the same as what the user has provided. But\nunfortunately we cannot assume that `realpath()` provides a canonical\nrepresentation of the path name, either.\n\n> Unless there is a demon that randomly flips the character case in a\n> pathname once it is obtained from the user or readdir() before it\n> gets to this code path, an easy way out may be to tell users \"don't\n> spell the pathnames inconsistently\" or its equivalent, \"do spell\n> them exactly the way readdir() would report on your system\", with \"if\n> you fail to do so, bad things will happen\".  I suspect that the bad\n> thing in this particular case is merely that a search in the\n> alternates is made unnecessarily inefficient due to duplicates, so it\n> may be a reasonable alternative.\n\nYeah. All of this is really just caused by the fact that there is no\nplatform-agnostic way to check whether two directories are the same\nthing. Which is kind of surprising, if you ask me.\n\n> Alternatively, we can even say \"your repository cannot span\n> filesystems with different case sensitivities\"; I am sure there\n> would be some users affected by such a declaration, but I do not\n> know how much we should care.\n\nI'm hesitant to go there, as that would retroactively introduce\nlimitations that could break existing use cases that we supported just\nfine until now. The proposed patch is carefully trying to not alter any\nuser-visible behaviour at all, so both before and after this patch the\nbehaviour with regards to case sensitivity should be exactly the same.\nAnd that current behaviour seems to be working just fine, or otherwise\nI assume we'd have seen bug reports in this area.\n\nSo I think we shouldn't throw the baby out with the bathwater.\n\n> > +/*\n> > + * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n> > + * _may_ be the same. This requires quite a bit of boilerplate for dubious\n> > + * benefit:\n> > + *\n> > + *   - Duplicating alternates should really only lead to regressed performance.\n> > + *\n> > + *   - We don't properly resolve symlinks or mointpoints, so we may still end\n> > + *     up duplicating alternates.\n> > + *\n> > + *   - The value may be lying, in which case we might deduplicate alternates\n> > + *     that are in fact not mapping to the same directory.\n> > + *\n> > + * We should investigate whether we can remove this whole mechanism outright.\n> > + */\n> > +static int odb_source_paths_cmp(struct object_database *o,\n> > +\t\t\t\tconst char *a, const char *b)\n> > +{\n> > +\tif (o->source_paths_icase < 0) {\n> > +\t\tint icase = 0;\n> > +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n> \n> I suspect accessing o->repo should be safe even in the\n> initialization sequence, simply because \"o->repo = repo\" is done as\n> the first thing in odb_new(), but do we know o->repo->initialized is\n> true in this code path?  Refraining from making that call and\n> assuming a case senstivie comparison may be necessary when o->repo\n> is not yet initialized.\n\nPart of the motivation for why I did all the refactorings in \"setup.c\"\nwas to ensure that we always set up the object database (and reference\ndatabase) after the repository was fully initialized, including all of\nits extensions. So yes, we know that it's fully initialized at this\npoint in time.\n\nOne way to prove this is by doing the following:\n\ndiff --git a/odb.c b/odb.c\nindex 115957e983..2f1cdfd592 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1063,6 +1063,9 @@ struct object_database *odb_new(struct repository *repo,\n \tchar *primary_source = NULL, *secondary_sources = NULL;\n \tstruct object_database *o;\n \n+\tif (!repo->initialized || !repo->commondir)\n+\t\tBUG(\"repository is not initialized\");\n+\n \tCALLOC_ARRAY(o, 1);\n \to->repo = repo;\n \tpthread_mutex_init(&o->replace_mutex, NULL);\n\nThat _does_ trigger a test failure, but only in our unit tests because\nwe don't fully initialize the environment there for t-odb-inmemory. All\nthe other tests are passing.\n\nI could add that to the patch series as a safety mechanism, but I'm not\nsure that's worth it.\n\nPatrick\n"},{"id":"551028","messageId":"xmqqik53qz5j.fsf@gitster.g","threadId":"66146","inReplyTo":"aof-t_bRzC0u1hHj@pks.im","subject":"Re: [PATCH v3 2/5] odb: decouple source path comparisons from `the_repository`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-21T15:19:36Z","receivedAt":"2026-08-21T15:19:39Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I don't feel comfortable removing that mechanism though. There's always\n> that one person relying on those weird edge cases.\n\nI do not, either, and more importantly, removing the hashing\nmechanism does not help an iota here to deal with case insensitive\nfilesystems.\n\n>> ... an easy way out may be to tell users \"don't\n>> spell the pathnames inconsistently\" or its equivalent, \"do spell\n>> them exactly the way readdir() would report on your system\", with \"if\n>> you fail to do so, bad things will happen\".  I suspect that the bad\n>> thing in this particular case is merely that a search in the\n>> alternates is made unnecessarily inefficient due to duplicates, so it\n>> may be a reasonable alternative.\n>\n> Yeah. All of this is really just caused by the fact that there is no\n> platform-agnostic way to check whether two directories are the same\n> thing. Which is kind of surprising, if you ask me.\n>\n>> Alternatively, we can even say \"your repository cannot span\n>> filesystems with different case sensitivities\"; I am sure there\n>> would be some users affected by such a declaration, but I do not\n>> know how much we should care.\n>\n> I'm hesitant to go there, as that would retroactively introduce\n> limitations that could break ...\n\nYup.  Which means the simplest way out would be to do a \"best\neffort\" case-insensitive match when there is a hint that the\nplatform might be using a case insensitive filesystem.\n\nAnd that in turn gives us a direction to solve this part ...\n\n>> > +/*\n>> > + * NEEDSWORK: we're using \"core.ignoreCase\" to deduplicate alternates that\n>> > + * _may_ be the same. This requires quite a bit of boilerplate for dubious\n>> > ...\n>> > +static int odb_source_paths_cmp(struct object_database *o,\n>> > +\t\t\t\tconst char *a, const char *b)\n>> > +{\n>> > +\tif (o->source_paths_icase < 0) {\n>> > +\t\tint icase = 0;\n>> > +\t\trepo_config_get_bool(o->repo, \"core.ignorecase\", &icase);\n>> \n>> I suspect accessing o->repo should be safe even in the\n>> initialization sequence, simply because \"o->repo = repo\" is done as\n>> the first thing in odb_new(), but do we know o->repo->initialized is\n>> true in this code path?  Refraining from making that call and\n>> assuming a case senstivie comparison may be necessary when o->repo\n>> is not yet initialized.\n\n...which is that, since case-insensitivity support is at most best\neffort, we do not really care if o->repo is not initialized.  The\ncode can stay as-is, and if the user spelled the path to a single\nalternate object store using two different cases in two places,\ncausing the code to treat them as two different entities, the effect\nis merely an extra search of the \"second copy\" (which is guaranteed\nto find nothing, after a search in the first copy finds no object\nthey are looking for)s, a minor performance penalty\n"}]}