* [PATCH 0/4] odb: eagerly load alternates
@ 2026-08-10 13:33 Patrick Steinhardt
2026-08-10 13:33 ` [PATCH 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-08-10 13:33 UTC (permalink / raw)
To: git
Hi,
when initializing the object database we only eagerly initialize the
primary object database source. If the primary source has alternates,
those alternates are only initialized the first time we really access
the object database.
When introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to
support multiple object databases., 2005-05-07), alternates were
originally only loaded when a given object wasn't found in the primary
object database. This was also reinforced by later optimization, for
example in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1,
2007-05-26), where we tried to avoid loading alternates in even more
cases. But as Git has evolved, we eventually started to eagerly parse
alternates all over the codebase, including on every single object
lookup, and consequently deferring this operation does not really buy us
much anymore.
The result of this is that we have calls to `odb_prepare_alternates()`
cluttered all over the code base. This is somewhat awkward, and as
almost every Git command ends up reading objects at it doesn't even buy
us anything.
This patch series thus gets rid of the lazy-loading. Besides simplifying
the codebase a bit, it also prepares us for moving alternates into the
"files" backend as discussed in [1].
The series is built on top of 010afd3166 (The 12th batch, 2026-08-07)
with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of
on-disk structures pluggable, 2026-08-07) merged into it.
Thanks!
Patrick
[1]: <amLgMqkqxR8mKIbT@pks.im>
---
Patrick Steinhardt (4):
odb: decouple source path comparisons from `the_repository`
odb: eagerly initialize alternates
odb: drop `loaded_alternates` field
odb: drop `alternates_db` field
builtin/fsck.c | 3 --
builtin/pack-objects.c | 3 --
commit-graph.c | 4 --
loose.c | 1 -
object-name.c | 1 -
odb.c | 106 ++++++++++++++++++++++++-------------------------
odb.h | 22 +++++-----
odb/source.h | 7 ++++
odb/streaming.c | 1 -
pack-bitmap.c | 2 -
packfile.c | 1 -
packfile.h | 2 -
12 files changed, 68 insertions(+), 85 deletions(-)
---
base-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765
change-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt @ 2026-08-10 13:33 ` Patrick Steinhardt 2026-08-11 22:04 ` Justin Tobler 2026-08-10 13:33 ` [PATCH 2/4] odb: eagerly initialize alternates Patrick Steinhardt ` (3 subsequent siblings) 4 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-10 13:33 UTC (permalink / raw) To: git When registering alternates we deduplicate object database sources by their path so that the same source won't be added twice. Ever since cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) this duplicate check is backed by a map keyed by the source's path, using `fspathhash()` and `fspatheq()` as hash and equality functions, respectively. These functions are problematic in this context for two reasons: - They implicitly depend on `the_repository` instead of the repository that owns the object database. - They derive case-sensitivity from `repo_ignore_case()`, which returns a default value in case the repository's configuration has not been parsed yet. Object database sources may be registered before that is the case, so the answer may flip depending on when a source gets registered. Fix this by making the comparison self-contained in the object database. Instead of using `fspathhash()` and `fspatheq()` we resolve "core.ignoreCase" manually and then use the correct comparison function based on the result. This requires us to migrate to a `struct hashmap`, as the khash interface does not give us the ability to change these functions. Note that we can unconditionally use `strihash()` to compute entry hashes regardless of case sensitivity: a hash function only needs to guarantee that equal keys have equal hashes, and a case-insensitive hash satisfies this requirement for both case-sensitive and case-insensitive equality. Overall it's quite debatable whether all of this complexity really is worth it, or whether we should just linearly search through all sources to find duplicates. But the mentioned commit cares about cases with thousands of alternates, and a linear search would of course regress performance quite a bit. This doesn't really feel like a reasonable case to care about though, but I don't feel comfortable regressing it anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 63 ++++++++++++++++++++++++++++++++++++++++-------------------- odb.h | 15 ++++++++++++++- odb/source.h | 7 +++++++ 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/odb.c b/odb.c index bd02d8ad54..51da386f22 100644 --- a/odb.c +++ b/odb.c @@ -2,11 +2,10 @@ #include "abspath.h" #include "commit-graph.h" #include "config.h" -#include "dir.h" #include "environment.h" #include "gettext.h" +#include "hashmap.h" #include "hex.h" -#include "khash.h" #include "lockfile.h" #include "loose.h" #include "midx.h" @@ -29,8 +28,32 @@ #include "trace2.h" #include "write-or-die.h" -KHASH_INIT(odb_path_map, const char * /* key: odb_path */, - struct odb_source *, 1, fspathhash, fspatheq) +static int odb_source_paths_cmp(struct object_database *o, + const char *a, const char *b) +{ + if (o->source_paths_icase < 0) { + int icase = 0; + repo_config_get_bool(o->repo, "core.ignorecase", &icase); + o->source_paths_icase = icase; + } + + return o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b); +} + +static int odb_source_by_path_cmp(const void *cb_data, + const struct hashmap_entry *entry, + const struct hashmap_entry *entry_or_key, + const void *keydata) +{ + struct object_database *o = (struct object_database *)cb_data; + const struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry); + const char *path = keydata; + + if (!path) + path = container_of(entry_or_key, const struct odb_source, by_path_entry)->path; + + return odb_source_paths_cmp(o, source->path, path); +} int odb_mkstemp(struct object_database *odb, struct strbuf *temp_filename, const char *pattern) @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb, */ static bool odb_is_source_usable(struct object_database *o, const char *path) { - int r; struct strbuf normalized_objdir = STRBUF_INIT; + struct hashmap_entry key; bool usable = false; strbuf_realpath(&normalized_objdir, o->sources->path, 1); @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path) * Prevent the common mistake of listing the same * thing twice, or object directory itself. */ - if (!o->source_by_path) { - khiter_t p; - - o->source_by_path = kh_init_odb_path_map(); + if (!hashmap_get_size(&o->source_by_path)) { assert(!o->sources->next); - p = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r); - assert(r == 1); /* never used */ - kh_value(o->source_by_path, p) = o->sources; + hashmap_entry_init(&o->sources->by_path_entry, + strihash(o->sources->path)); + hashmap_add(&o->source_by_path, &o->sources->by_path_entry); } - if (fspatheq(path, normalized_objdir.buf)) + if (!odb_source_paths_cmp(o, path, normalized_objdir.buf)) goto out; - if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path)) + hashmap_entry_init(&key, strihash(path)); + if (hashmap_get(&o->source_by_path, &key, path)) goto out; usable = true; @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * { struct odb_source *alternate = NULL; struct strvec sources = STRVEC_INIT; - khiter_t pos; - int ret; if (!odb_is_source_usable(odb, source)) goto error; @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * *odb->sources_tail = alternate; odb->sources_tail = &(alternate->next); - pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret); - if (!ret) + hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path)); + if (hashmap_get(&odb->source_by_path, &alternate->by_path_entry, + alternate->path)) BUG("source must not yet exist"); - kh_value(odb->source_by_path, pos) = alternate; + hashmap_add(&odb->source_by_path, &alternate->by_path_entry); /* recursively add alternates */ odb_source_read_alternates(alternate, &sources); @@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo, o->repo = repo; pthread_mutex_init(&o->replace_mutex, NULL); string_list_init_dup(&o->submodule_source_paths); + hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0); + o->source_paths_icase = -1; if (flags & ODB_NEW_HONOR_ENV) { primary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT)); @@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o) odb_source_free(o->inmemory_objects); o->inmemory_objects = NULL; - kh_destroy_odb_path_map(o->source_by_path); - o->source_by_path = NULL; + hashmap_clear(&o->source_by_path); } void odb_free(struct object_database *o) diff --git a/odb.h b/odb.h index 8eb4e85d64..71af7450a9 100644 --- a/odb.h +++ b/odb.h @@ -1,6 +1,7 @@ #ifndef ODB_H #define ODB_H +#include "hashmap.h" #include "object.h" #include "oidset.h" #include "oidmap.h" @@ -54,7 +55,19 @@ struct object_database { */ struct odb_source *sources; struct odb_source **sources_tail; - struct kh_odb_path_map *source_by_path; + + /* + * Map of object database sources, keyed by their respective paths. + * This map is used to detect the case where the same source is + * registered multiple times. + */ + struct hashmap source_by_path; + + /* + * Whether source paths shall be compared case-insensitively, as + * determined by "core.ignoreCase". + */ + int source_paths_icase; int loaded_alternates; diff --git a/odb/source.h b/odb/source.h index 4bc037b8d6..82cda8ad75 100644 --- a/odb/source.h +++ b/odb/source.h @@ -1,6 +1,7 @@ #ifndef ODB_SOURCE_H #define ODB_SOURCE_H +#include "hashmap.h" #include "object.h" #include "odb.h" #include "odb/transaction.h" @@ -50,6 +51,12 @@ struct strvec; struct odb_source { struct odb_source *next; + /* + * Entry in the object database's map of sources, keyed by this + * source's path. + */ + struct hashmap_entry by_path_entry; + /* Object database that owns this object source. */ struct object_database *odb; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-10 13:33 ` [PATCH 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt @ 2026-08-11 22:04 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 1 reply; 23+ messages in thread From: Justin Tobler @ 2026-08-11 22:04 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/10 03:33PM, Patrick Steinhardt wrote: > When registering alternates we deduplicate object database sources by > their path so that the same source won't be added twice. Ever since > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) > this duplicate check is backed by a map keyed by the source's path, > using `fspathhash()` and `fspatheq()` as hash and equality functions, > respectively. > > These functions are problematic in this context for two reasons: > > - They implicitly depend on `the_repository` instead of the > repository that owns the object database. > > - They derive case-sensitivity from `repo_ignore_case()`, which > returns a default value in case the repository's configuration has > not been parsed yet. Object database sources may be registered > before that is the case, so the answer may flip depending on when a > source gets registered. Are alternates currently always registered after repository configuration has been parsed? Or is this an existing bug? > Fix this by making the comparison self-contained in the object > database. Instead of using `fspathhash()` and `fspatheq()` we resolve > "core.ignoreCase" manually and then use the correct comparison function > based on the result. This requires us to migrate to a `struct hashmap`, > as the khash interface does not give us the ability to change these > functions. > > Note that we can unconditionally use `strihash()` to compute entry > hashes regardless of case sensitivity: a hash function only needs to > guarantee that equal keys have equal hashes, and a case-insensitive > hash satisfies this requirement for both case-sensitive and > case-insensitive equality. Ok IIUC, even if we want to be case-sensitive, its ok to use `strihash()` and have hash collisions because the compare function will still properly distinguish between the cases. Makes sense. > Overall it's quite debatable whether all of this complexity really is > worth it, or whether we should just linearly search through all sources > to find duplicates. But the mentioned commit cares about cases with > thousands of alternates, and a linear search would of course regress > performance quite a bit. This doesn't really feel like a reasonable case > to care about though, but I don't feel comfortable regressing it anyway. Ya, my first though here was also whether all of this song and dance is really needed for alternates. There may be someone out there with tons of alternates I guess though. Probably good to be on the safe side. > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > odb.c | 63 ++++++++++++++++++++++++++++++++++++++++-------------------- > odb.h | 15 ++++++++++++++- > odb/source.h | 7 +++++++ > 3 files changed, 63 insertions(+), 22 deletions(-) > > diff --git a/odb.c b/odb.c > index bd02d8ad54..51da386f22 100644 > --- a/odb.c > +++ b/odb.c > @@ -2,11 +2,10 @@ > #include "abspath.h" > #include "commit-graph.h" > #include "config.h" > -#include "dir.h" > #include "environment.h" > #include "gettext.h" > +#include "hashmap.h" > #include "hex.h" > -#include "khash.h" > #include "lockfile.h" > #include "loose.h" > #include "midx.h" > @@ -29,8 +28,32 @@ > #include "trace2.h" > #include "write-or-die.h" > > -KHASH_INIT(odb_path_map, const char * /* key: odb_path */, > - struct odb_source *, 1, fspathhash, fspatheq) > +static int odb_source_paths_cmp(struct object_database *o, > + const char *a, const char *b) > +{ > + if (o->source_paths_icase < 0) { > + int icase = 0; > + repo_config_get_bool(o->repo, "core.ignorecase", &icase); > + o->source_paths_icase = icase; > + } We now parse ignorecase configuration here directly and store the result in `source_paths_icase`. This ensures configuration is correctly applied regardless of whether repository configuration has been fully read yet. > + return o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b); > +} > + > +static int odb_source_by_path_cmp(const void *cb_data, > + const struct hashmap_entry *entry, > + const struct hashmap_entry *entry_or_key, > + const void *keydata) > +{ > + struct object_database *o = (struct object_database *)cb_data; > + const struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry); > + const char *path = keydata; > + > + if (!path) > + path = container_of(entry_or_key, const struct odb_source, by_path_entry)->path; > + > + return odb_source_paths_cmp(o, source->path, path); > +} Here is the comparison callback that is used for the hashmap. > int odb_mkstemp(struct object_database *odb, > struct strbuf *temp_filename, const char *pattern) > @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb, > */ > static bool odb_is_source_usable(struct object_database *o, const char *path) > { > - int r; > struct strbuf normalized_objdir = STRBUF_INIT; > + struct hashmap_entry key; > bool usable = false; > > strbuf_realpath(&normalized_objdir, o->sources->path, 1); > @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path) > * Prevent the common mistake of listing the same > * thing twice, or object directory itself. > */ > - if (!o->source_by_path) { > - khiter_t p; > - > - o->source_by_path = kh_init_odb_path_map(); > + if (!hashmap_get_size(&o->source_by_path)) { > assert(!o->sources->next); > - p = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r); > - assert(r == 1); /* never used */ > - kh_value(o->source_by_path, p) = o->sources; > + hashmap_entry_init(&o->sources->by_path_entry, > + strihash(o->sources->path)); > + hashmap_add(&o->source_by_path, &o->sources->by_path_entry); The hashmap is lazily set up with the primary source. I do find some of the variable names like "source_by_path" a bit vague, but that isn't really anything new here. > } > > - if (fspatheq(path, normalized_objdir.buf)) > + if (!odb_source_paths_cmp(o, path, normalized_objdir.buf)) > goto out; If the path matches the first entry in the sources list then we know it is not an alternate. > > - if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path)) > + hashmap_entry_init(&key, strihash(path)); > + if (hashmap_get(&o->source_by_path, &key, path)) > goto out; If the alternates source cannot be found for the given path, then we also know it is not a usuable alternate. > usable = true; > @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * > { > struct odb_source *alternate = NULL; > struct strvec sources = STRVEC_INIT; > - khiter_t pos; > - int ret; > > if (!odb_is_source_usable(odb, source)) > goto error; > @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * > *odb->sources_tail = alternate; > odb->sources_tail = &(alternate->next); > > - pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret); > - if (!ret) > + hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path)); > + if (hashmap_get(&odb->source_by_path, &alternate->by_path_entry, > + alternate->path)) > BUG("source must not yet exist"); > - kh_value(odb->source_by_path, pos) = alternate; > + hashmap_add(&odb->source_by_path, &alternate->by_path_entry); Here is where alternates get registered and added to the hashmap. Makes sense. Overall this patch looks good. -Justin ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-11 22:04 ` Justin Tobler @ 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 5:39 UTC (permalink / raw) To: Justin Tobler; +Cc: git On Tue, Aug 11, 2026 at 05:04:58PM -0500, Justin Tobler wrote: > On 26/08/10 03:33PM, Patrick Steinhardt wrote: > > When registering alternates we deduplicate object database sources by > > their path so that the same source won't be added twice. Ever since > > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) > > this duplicate check is backed by a map keyed by the source's path, > > using `fspathhash()` and `fspatheq()` as hash and equality functions, > > respectively. > > > > These functions are problematic in this context for two reasons: > > > > - They implicitly depend on `the_repository` instead of the > > repository that owns the object database. > > > > - They derive case-sensitivity from `repo_ignore_case()`, which > > returns a default value in case the repository's configuration has > > not been parsed yet. Object database sources may be registered > > before that is the case, so the answer may flip depending on when a > > source gets registered. > > Are alternates currently always registered after repository > configuration has been parsed? Or is this an existing bug? They are, because of the lazy-loading. So this is not a bug, we merely have to ensure that we retain this behaviour. > > Overall it's quite debatable whether all of this complexity really is > > worth it, or whether we should just linearly search through all sources > > to find duplicates. But the mentioned commit cares about cases with > > thousands of alternates, and a linear search would of course regress > > performance quite a bit. This doesn't really feel like a reasonable case > > to care about though, but I don't feel comfortable regressing it anyway. > > Ya, my first though here was also whether all of this song and dance is > really needed for alternates. There may be someone out there with tons > of alternates I guess though. Probably good to be on the safe side. cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) mentions a repository with 100k alternates in total, but that's an artificial testing setup. I doubt you can get any kind of reasonable performance out of such a repository, regardless of whether on not parsing the alternates is going to be fast. For now though I didn't want to remove this infra. It feels overblown, but it's not an unmaintainable mess, either. Patrick ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/4] odb: eagerly initialize alternates 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt @ 2026-08-10 13:33 ` Patrick Steinhardt 2026-08-11 22:15 ` Justin Tobler 2026-08-10 13:33 ` [PATCH 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt ` (2 subsequent siblings) 4 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-10 13:33 UTC (permalink / raw) To: git When creating the object database we initialize the main object database source, but we don't yet initialize its alternates. Instead, we have many calls to `odb_prepare_alternates()` cluttered around the code base whenever we are about to iterate through the sources. This lazy loading doesn't really add much value: the moment where read any object we _have_ to load the alternates anyway. So given that most of our commands would access the object database this optimization is not really buying us much in the first place. Quite on the contrary, it makes the code harder to understand and is a potential source of bugs in case any callsite forgot to prepare alternates before we iterate through the sources. Historically though there was a reason why we deferred lazy-loading: it may happen that the repository has "core.ignoreCase" configured, and we use that to deduplicate the list of alternates in case we had the same alternate configured multiple times, but with different casing. We used to initialize the object database before we had fully configured the owning repository though, and consequently we couldn't access that configuration yet. This has changed in the preceding commit though where we started to parse "core.ignoreCase" manually. Eagerly prepare alternates both when creating the object database and when flushing its caches. Drop the now-unneeded calls to prepare the alternates that are scattered across the code base. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/fsck.c | 3 --- builtin/pack-objects.c | 3 --- commit-graph.c | 4 ---- loose.c | 1 - object-name.c | 1 - odb.c | 26 ++++---------------------- odb.h | 6 ------ odb/streaming.c | 1 - pack-bitmap.c | 2 -- packfile.c | 1 - packfile.h | 2 -- 11 files changed, 4 insertions(+), 46 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index a6c054e45b..892c5661d9 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -1069,7 +1069,6 @@ int cmd_fsck(int argc, odb_for_each_object(repo->objects, NULL, mark_object_for_connectivity, repo, 0); } else { - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) fsck_source(repo, source); @@ -1155,7 +1154,6 @@ int cmd_fsck(int argc, if (repo->settings.core_commit_graph) { struct child_process commit_graph_verify = CHILD_PROCESS_INIT; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { child_process_init(&commit_graph_verify); commit_graph_verify.git_cmd = 1; @@ -1173,7 +1171,6 @@ int cmd_fsck(int argc, if (repo->settings.core_multi_pack_index) { struct child_process midx_verify = CHILD_PROCESS_INIT; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { child_process_init(&midx_verify); midx_verify.git_cmd = 1; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1ec5b6f206..48d37e8e32 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1779,8 +1779,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid, *found_offset = 0; } - odb_prepare_alternates(the_repository->objects); - for (source = the_repository->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct multi_pack_index *m = get_multi_pack_index(files->packed); @@ -4520,7 +4518,6 @@ static void add_objects_in_unpacked_packs(void) .source_infop = &source_info, }; - odb_prepare_alternates(to_pack.repo->objects); for (source = to_pack.repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); diff --git a/commit-graph.c b/commit-graph.c index 49e8f63930..983c11ce85 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -651,8 +651,6 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, count = st->st_size / (odb->repo->hash_algo->hexsz + 1); CALLOC_ARRAY(oids, count); - odb_prepare_alternates(odb); - for (i = 0; i < count; i++) { struct odb_source *source; @@ -768,7 +766,6 @@ static struct commit_graph *prepare_commit_graph(struct repository *r) if (!commit_graph_compatible(r)) return NULL; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { r->objects->commit_graph = read_commit_graph_one(source); if (r->objects->commit_graph) @@ -2018,7 +2015,6 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx) _("Finding commits for commit graph among packed objects"), ctx->approx_nr_objects); - odb_prepare_alternates(ctx->r->objects); for (source = ctx->r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); odb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi, diff --git a/loose.c b/loose.c index aa3cb1b4fc..c159d29d2d 100644 --- a/loose.c +++ b/loose.c @@ -115,7 +115,6 @@ int repo_read_loose_object_map(struct repository *repo) { struct odb_source *source; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); if (loose_object_map_load(files->loose) < 0) diff --git a/object-name.c b/object-name.c index 83efba0ba6..34a08d76dd 100644 --- a/object-name.c +++ b/object-name.c @@ -280,7 +280,6 @@ static int init_object_disambiguation(struct repository *r, ds->len = len; ds->repo = r; - odb_prepare_alternates(r->objects); return 0; } diff --git a/odb.c b/odb.c index 51da386f22..2ae8228dd2 100644 --- a/odb.c +++ b/odb.c @@ -237,11 +237,6 @@ void odb_add_to_alternates_file(struct object_database *odb, struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, const char *dir) { - /* - * Make sure alternates are initialized, or else our entry may be - * overwritten when they are. - */ - odb_prepare_alternates(odb); return odb_add_alternate_recursively(odb, dir, 0); } @@ -250,12 +245,6 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, { struct odb_source *source; - /* - * Make sure alternates are initialized, or else our entry may be - * overwritten when they are. - */ - odb_prepare_alternates(odb); - /* * Make a new primary odb and link the old primary ODB in as an * alternate @@ -361,7 +350,6 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_ char *obj_dir_real = real_pathdup(obj_dir, 1); struct strbuf odb_path_real = STRBUF_INIT; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) { strbuf_realpath(&odb_path_real, source->path, 1); if (!strcmp(obj_dir_real, odb_path_real.buf)) @@ -495,7 +483,6 @@ int odb_for_each_alternate(struct object_database *odb, struct odb_source *alternate; int r = 0; - odb_prepare_alternates(odb); for (alternate = odb->sources->next; alternate; alternate = alternate->next) { r = cb(alternate, payload); if (r) @@ -504,7 +491,7 @@ int odb_for_each_alternate(struct object_database *odb, return r; } -void odb_prepare_alternates(struct object_database *odb) +static void odb_prepare_alternates(struct object_database *odb) { struct strvec sources = STRVEC_INIT; @@ -523,7 +510,6 @@ void odb_prepare_alternates(struct object_database *odb) int odb_has_alternates(struct object_database *odb) { - odb_prepare_alternates(odb); return !!odb->sources->next; } @@ -583,8 +569,6 @@ static int do_oid_object_info_extended(struct object_database *odb, if (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags)) return 0; - odb_prepare_alternates(odb); - while (1) { struct odb_source *source; @@ -847,7 +831,6 @@ int odb_freshen_object(struct object_database *odb, const struct object_id *oid) { struct odb_source *source; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) if (odb_source_freshen_object(source, oid, NULL)) return 1; @@ -862,7 +845,6 @@ int odb_for_each_object_ext(struct object_database *odb, { int ret; - odb_prepare_alternates(odb); for (struct odb_source *source = odb->sources; source; source = source->next) { if (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local) continue; @@ -900,7 +882,6 @@ int odb_count_objects(struct object_database *odb, return 0; } - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) { unsigned long c; @@ -980,7 +961,6 @@ int odb_find_abbrev_len(struct object_database *odb, goto out; } - odb_prepare_alternates(odb); for (struct odb_source *source = odb->sources; source; source = source->next) { ret = odb_source_find_abbrev_len(source, oid, len, &len); if (ret) @@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo, o->alternate_db = secondary_sources; o->inmemory_objects = &odb_source_inmemory_new(o)->base; + odb_prepare_alternates(o); + free(primary_source); return o; } @@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) */ if (flags & ODB_PREPARE_FLUSH_CACHES) { o->loaded_alternates = 0; + odb_prepare_alternates(o); o->object_count_valid = 0; } - odb_prepare_alternates(o); for (source = o->sources; source; source = source->next) odb_source_prepare(source, flags); diff --git a/odb.h b/odb.h index 71af7450a9..fbafee174b 100644 --- a/odb.h +++ b/odb.h @@ -273,12 +273,6 @@ void odb_for_each_alternate_ref(struct object_database *odb, int odb_mkstemp(struct object_database *odb, struct strbuf *temp_filename, const char *pattern); -/* - * Prepare alternate object sources for the given database by reading - * "objects/info/alternates" and opening the respective sources. - */ -void odb_prepare_alternates(struct object_database *odb); - /* * Check whether the object database has any alternates. The primary object * source does not count as alternate. diff --git a/odb/streaming.c b/odb/streaming.c index 20531e864c..37642768e9 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -184,7 +184,6 @@ static int istream_source(struct odb_read_stream **out, { struct odb_source *source; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) if (!odb_source_read_object_stream(out, source, oid)) return 0; diff --git a/pack-bitmap.c b/pack-bitmap.c index e85bd69ba4..e0fb57d332 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -717,7 +717,6 @@ static int open_bitmap(struct repository *r, assert(!bitmap_git->map); - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); @@ -3417,7 +3416,6 @@ int verify_bitmap_files(struct repository *r) struct packed_git *p; int res = 0; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct multi_pack_index *m = get_multi_pack_index(files->packed); diff --git a/packfile.c b/packfile.c index 0eee45055f..d870de90ed 100644 --- a/packfile.c +++ b/packfile.c @@ -1938,7 +1938,6 @@ int has_object_pack(struct repository *r, const struct object_id *oid) { struct odb_source *source; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); if (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0)) diff --git a/packfile.h b/packfile.h index e1f77152b5..10de24f477 100644 --- a/packfile.h +++ b/packfile.h @@ -77,8 +77,6 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct { struct repo_for_each_pack_data data = { 0 }; - odb_prepare_alternates(repo->objects); - for (struct odb_source *source = repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct packfile_list_entry *entry = packfile_store_get_packs(files->packed); -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] odb: eagerly initialize alternates 2026-08-10 13:33 ` [PATCH 2/4] odb: eagerly initialize alternates Patrick Steinhardt @ 2026-08-11 22:15 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 1 reply; 23+ messages in thread From: Justin Tobler @ 2026-08-11 22:15 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/10 03:33PM, Patrick Steinhardt wrote: > When creating the object database we initialize the main object database > source, but we don't yet initialize its alternates. Instead, we have > many calls to `odb_prepare_alternates()` cluttered around the code base > whenever we are about to iterate through the sources. > > This lazy loading doesn't really add much value: the moment where read Should this say "where we read" instead? > any object we _have_ to load the alternates anyway. So given that most > of our commands would access the object database this optimization is > not really buying us much in the first place. Quite on the contrary, it > makes the code harder to understand and is a potential source of bugs in > case any callsite forgot to prepare alternates before we iterate through > the sources. > > Historically though there was a reason why we deferred lazy-loading: it > may happen that the repository has "core.ignoreCase" configured, and we > use that to deduplicate the list of alternates in case we had the same > alternate configured multiple times, but with different casing. We used > to initialize the object database before we had fully configured the > owning repository though, and consequently we couldn't access that > configuration yet. This has changed in the preceding commit though where > we started to parse "core.ignoreCase" manually. > > Eagerly prepare alternates both when creating the object database and > when flushing its caches. Drop the now-unneeded calls to prepare the > alternates that are scattered across the code base. This sounds like the right direction and overall much simpler. Nice. > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- [snip] > @@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo, > o->alternate_db = secondary_sources; > o->inmemory_objects = &odb_source_inmemory_new(o)->base; > > + odb_prepare_alternates(o); Now we eagerly prepare alternates at time of ODB creation. > + > free(primary_source); > return o; > } > @@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > */ > if (flags & ODB_PREPARE_FLUSH_CACHES) { > o->loaded_alternates = 0; > + odb_prepare_alternates(o); > o->object_count_valid = 0; > } Ok we also invoke `odb_prepare_alternates()` when we need to refresh all alternate sources. Makes sense. The rest of this patch just removes the now-unneeded `odb_prepare_alternates()` call sites and looks good. -Justin ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] odb: eagerly initialize alternates 2026-08-11 22:15 ` Justin Tobler @ 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 5:39 UTC (permalink / raw) To: Justin Tobler; +Cc: git On Tue, Aug 11, 2026 at 05:15:33PM -0500, Justin Tobler wrote: > On 26/08/10 03:33PM, Patrick Steinhardt wrote: > > When creating the object database we initialize the main object database > > source, but we don't yet initialize its alternates. Instead, we have > > many calls to `odb_prepare_alternates()` cluttered around the code base > > whenever we are about to iterate through the sources. > > > > This lazy loading doesn't really add much value: the moment where read > > Should this say "where we read" instead? Yes, indeed. Patrick ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/4] odb: drop `loaded_alternates` field 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 2/4] odb: eagerly initialize alternates Patrick Steinhardt @ 2026-08-10 13:33 ` Patrick Steinhardt 2026-08-11 22:22 ` Justin Tobler 2026-08-10 13:33 ` [PATCH 4/4] odb: drop `alternates_db` field Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt 4 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-10 13:33 UTC (permalink / raw) To: git The `struct object_database::loaded_alternates` field tells us whether or not alternates have been loaded already. This field was useful before the preceding commit as we were indeed lazy-loading alternates. But now that we started to eagerly load them we can assume them to be loaded after `odb_new()`, and hence the field does not serve any purpose anymore. Remove it. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 9 +-------- odb.h | 2 -- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/odb.c b/odb.c index 2ae8228dd2..2eb37a2f44 100644 --- a/odb.c +++ b/odb.c @@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb, int ret = odb_source_write_alternate(odb->sources, dir); if (ret < 0) die(NULL); - if (odb->loaded_alternates) - odb_add_alternate_recursively(odb, dir, 0); + odb_add_alternate_recursively(odb, dir, 0); } struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, @@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb) { struct strvec sources = STRVEC_INIT; - if (odb->loaded_alternates) - return; - parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources); odb_source_read_alternates(odb->sources, &sources); for (size_t i = 0; i < sources.nr; i++) odb_add_alternate_recursively(odb, sources.v[i], 0); - odb->loaded_alternates = 1; - strvec_clear(&sources); } @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) * the lifetime of the process. */ if (flags & ODB_PREPARE_FLUSH_CACHES) { - o->loaded_alternates = 0; odb_prepare_alternates(o); o->object_count_valid = 0; } diff --git a/odb.h b/odb.h index fbafee174b..aefb34213f 100644 --- a/odb.h +++ b/odb.h @@ -69,8 +69,6 @@ struct object_database { */ int source_paths_icase; - int loaded_alternates; - /* * A list of alternate object directories loaded from the environment; * this should not generally need to be accessed directly, but will -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 3/4] odb: drop `loaded_alternates` field 2026-08-10 13:33 ` [PATCH 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt @ 2026-08-11 22:22 ` Justin Tobler 0 siblings, 0 replies; 23+ messages in thread From: Justin Tobler @ 2026-08-11 22:22 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/10 03:33PM, Patrick Steinhardt wrote: > The `struct object_database::loaded_alternates` field tells us whether > or not alternates have been loaded already. This field was useful before > the preceding commit as we were indeed lazy-loading alternates. But now > that we started to eagerly load them we can assume them to be loaded > after `odb_new()`, and hence the field does not serve any purpose > anymore. Now that alternates are eagerly set up, it is safe to assume, if we have an ODB, the alternates have been loaded. Makes sense. > Remove it. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- [snip] > @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > * the lifetime of the process. > */ > if (flags & ODB_PREPARE_FLUSH_CACHES) { > - o->loaded_alternates = 0; > odb_prepare_alternates(o); Also nice to see this go away as I thought it was little bit awkward to unset it just to allow the us to reprepare the alternates. -Justin ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/4] odb: drop `alternates_db` field 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt ` (2 preceding siblings ...) 2026-08-10 13:33 ` [PATCH 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt @ 2026-08-10 13:33 ` Patrick Steinhardt 2026-08-11 22:31 ` Justin Tobler 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt 4 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-10 13:33 UTC (permalink / raw) To: git The `struct object_database::alternates_db` field tracks the value of the "GIT_ALTERNATE_OBJECT_DIRECTORIES" environment variable and is used in `odb_prepare_alternates()`. It's not necessary to store it as a separate field anymore though, as we stopped lazy-loading alternates. Consequently, we can simply pass it to `odb_prepare_alternates()` via `odb_new()` now. Do so and remove the field. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 14 +++++++------- odb.h | 7 ------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/odb.c b/odb.c index 2eb37a2f44..fc21199f80 100644 --- a/odb.c +++ b/odb.c @@ -490,12 +490,14 @@ int odb_for_each_alternate(struct object_database *odb, return r; } -static void odb_prepare_alternates(struct object_database *odb) +static void odb_prepare_alternates(struct object_database *odb, + const char *alternate_db) { struct strvec sources = STRVEC_INIT; - parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources); + parse_alternates(alternate_db, PATH_SEP, NULL, &sources); odb_source_read_alternates(odb->sources, &sources); + for (size_t i = 0; i < sources.nr; i++) odb_add_alternate_recursively(odb, sources.v[i], 0); @@ -1062,11 +1064,11 @@ struct object_database *odb_new(struct repository *repo, o->sources = odb_source_new(o, primary_source, true); o->sources_tail = &o->sources->next; - o->alternate_db = secondary_sources; o->inmemory_objects = &odb_source_inmemory_new(o)->base; - odb_prepare_alternates(o); + odb_prepare_alternates(o, secondary_sources); + free(secondary_sources); free(primary_source); return o; } @@ -1100,8 +1102,6 @@ void odb_free(struct object_database *o) if (!o) return; - free(o->alternate_db); - oidmap_clear(&o->replace_map, 1); pthread_mutex_destroy(&o->replace_mutex); @@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) * the lifetime of the process. */ if (flags & ODB_PREPARE_FLUSH_CACHES) { - odb_prepare_alternates(o); + odb_prepare_alternates(o, NULL); o->object_count_valid = 0; } diff --git a/odb.h b/odb.h index aefb34213f..748366a610 100644 --- a/odb.h +++ b/odb.h @@ -69,13 +69,6 @@ struct object_database { */ int source_paths_icase; - /* - * A list of alternate object directories loaded from the environment; - * this should not generally need to be accessed directly, but will - * populate the "sources" list when odb_prepare_alternates() is run. - */ - char *alternate_db; - /* * Objects that should be substituted by other objects * (see git-replace(1)). -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] odb: drop `alternates_db` field 2026-08-10 13:33 ` [PATCH 4/4] odb: drop `alternates_db` field Patrick Steinhardt @ 2026-08-11 22:31 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 1 reply; 23+ messages in thread From: Justin Tobler @ 2026-08-11 22:31 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 26/08/10 03:33PM, Patrick Steinhardt wrote: > The `struct object_database::alternates_db` field tracks the value of > the "GIT_ALTERNATE_OBJECT_DIRECTORIES" environment variable and is > used in `odb_prepare_alternates()`. It's not necessary to store it as a > separate field anymore though, as we stopped lazy-loading alternates. > Consequently, we can simply pass it to `odb_prepare_alternates()` via > `odb_new()` now. > > Do so and remove the field. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- [snip] > @@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > * the lifetime of the process. > */ > if (flags & ODB_PREPARE_FLUSH_CACHES) { > - odb_prepare_alternates(o); > + odb_prepare_alternates(o, NULL); > o->object_count_valid = 0; > } Naive question: is the reason we don't need to wire the `GIT_ALTERNATE_OBJECT_DIRECTORIES` environment variable here because they have already been added as sources? IOW, when we invoke `odb_prepare_alternates()` after the initial set up, we only really care about re-reading the alternates file. -Justin ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] odb: drop `alternates_db` field 2026-08-11 22:31 ` Justin Tobler @ 2026-08-12 5:39 ` Patrick Steinhardt 0 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 5:39 UTC (permalink / raw) To: Justin Tobler; +Cc: git On Tue, Aug 11, 2026 at 05:31:17PM -0500, Justin Tobler wrote: > On 26/08/10 03:33PM, Patrick Steinhardt wrote: > > The `struct object_database::alternates_db` field tracks the value of > > the "GIT_ALTERNATE_OBJECT_DIRECTORIES" environment variable and is > > used in `odb_prepare_alternates()`. It's not necessary to store it as a > > separate field anymore though, as we stopped lazy-loading alternates. > > Consequently, we can simply pass it to `odb_prepare_alternates()` via > > `odb_new()` now. > > > > Do so and remove the field. > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > [snip] > > @@ -1126,7 +1126,7 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > > * the lifetime of the process. > > */ > > if (flags & ODB_PREPARE_FLUSH_CACHES) { > > - odb_prepare_alternates(o); > > + odb_prepare_alternates(o, NULL); > > o->object_count_valid = 0; > > } > > Naive question: is the reason we don't need to wire the > `GIT_ALTERNATE_OBJECT_DIRECTORIES` environment variable here because > they have already been added as sources? IOW, when we invoke > `odb_prepare_alternates()` after the initial set up, we only really care > about re-reading the alternates file. Yes, exactly. We set up alternates exactly once in `odb_new()`, and we don't expect the environment variable to ever change in a running process. And as `odb_prepare_alternates()` only adds but never removes any it's fine to ignore those here. I'll add a comment. Patrick ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 0/4] odb: eagerly load alternates 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt ` (3 preceding siblings ...) 2026-08-10 13:33 ` [PATCH 4/4] odb: drop `alternates_db` field Patrick Steinhardt @ 2026-08-12 9:13 ` Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt ` (5 more replies) 4 siblings, 6 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 9:13 UTC (permalink / raw) To: git; +Cc: Justin Tobler Hi, when initializing the object database we only eagerly initialize the primary object database source. If the primary source has alternates, those alternates are only initialized the first time we really access the object database. When introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to support multiple object databases., 2005-05-07), alternates were originally only loaded when a given object wasn't found in the primary object database. This was also reinforced by later optimization, for example in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1, 2007-05-26), where we tried to avoid loading alternates in even more cases. But as Git has evolved, we eventually started to eagerly parse alternates all over the codebase, including on every single object lookup, and consequently deferring this operation does not really buy us much anymore. The result of this is that we have calls to `odb_prepare_alternates()` cluttered all over the code base. This is somewhat awkward, and as almost every Git command ends up reading objects at it doesn't even buy us anything. This patch series thus gets rid of the lazy-loading. Besides simplifying the codebase a bit, it also prepares us for moving alternates into the "files" backend as discussed in [1]. The series is built on top of 010afd3166 (The 12th batch, 2026-08-07) with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of on-disk structures pluggable, 2026-08-07) merged into it. Changes in v2: - Add a missing word to a commit message. - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES when re-preparing the object database. - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im Thanks! Patrick [1]: <amLgMqkqxR8mKIbT@pks.im> --- Patrick Steinhardt (4): odb: decouple source path comparisons from `the_repository` odb: eagerly initialize alternates odb: drop `loaded_alternates` field odb: drop `alternates_db` field builtin/fsck.c | 3 -- builtin/pack-objects.c | 3 -- commit-graph.c | 4 -- loose.c | 1 - object-name.c | 1 - odb.c | 109 ++++++++++++++++++++++++------------------------- odb.h | 22 +++++----- odb/source.h | 7 ++++ odb/streaming.c | 1 - pack-bitmap.c | 2 - packfile.c | 1 - packfile.h | 2 - 12 files changed, 70 insertions(+), 86 deletions(-) Range-diff versus v1: 1: 25802adffa = 1: 721907c60d odb: decouple source path comparisons from `the_repository` 2: 1e73b730d8 ! 2: 3b2c23566c odb: eagerly initialize alternates @@ Commit message many calls to `odb_prepare_alternates()` cluttered around the code base whenever we are about to iterate through the sources. - This lazy loading doesn't really add much value: the moment where read - any object we _have_ to load the alternates anyway. So given that most - of our commands would access the object database this optimization is - not really buying us much in the first place. Quite on the contrary, it - makes the code harder to understand and is a potential source of bugs in - case any callsite forgot to prepare alternates before we iterate through - the sources. + This lazy loading doesn't really add much value: the moment where we + read any object we _have_ to load the alternates anyway. So given that + most of our commands would access the object database this optimization + is not really buying us much in the first place. Quite on the contrary, + it makes the code harder to understand and is a potential source of bugs + in case any callsite forgot to prepare alternates before we iterate + through the sources. Historically though there was a reason why we deferred lazy-loading: it may happen that the repository has "core.ignoreCase" configured, and we 3: 2ca1aa2a37 = 3: df5d7df91d odb: drop `loaded_alternates` field 4: 1e97c93bdf ! 4: 50a37ef385 odb: drop `alternates_db` field @@ odb.c: void odb_free(struct object_database *o) pthread_mutex_destroy(&o->replace_mutex); @@ odb.c: void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) - * the lifetime of the process. + * Reprepare alt odbs, in case the alternates file was modified + * during the course of this process. This only _adds_ odbs to + * the linked list, so existing odbs will continue to exist for +- * the lifetime of the process. ++ * the lifetime of the process. Consequently, we don't have to ++ * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here. */ if (flags & ODB_PREPARE_FLUSH_CACHES) { - odb_prepare_alternates(o); --- base-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765 change-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt @ 2026-08-12 9:13 ` Patrick Steinhardt 2026-08-13 12:23 ` Karthik Nayak 2026-08-12 9:13 ` [PATCH v2 2/4] odb: eagerly initialize alternates Patrick Steinhardt ` (4 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 9:13 UTC (permalink / raw) To: git; +Cc: Justin Tobler When registering alternates we deduplicate object database sources by their path so that the same source won't be added twice. Ever since cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) this duplicate check is backed by a map keyed by the source's path, using `fspathhash()` and `fspatheq()` as hash and equality functions, respectively. These functions are problematic in this context for two reasons: - They implicitly depend on `the_repository` instead of the repository that owns the object database. - They derive case-sensitivity from `repo_ignore_case()`, which returns a default value in case the repository's configuration has not been parsed yet. Object database sources may be registered before that is the case, so the answer may flip depending on when a source gets registered. Fix this by making the comparison self-contained in the object database. Instead of using `fspathhash()` and `fspatheq()` we resolve "core.ignoreCase" manually and then use the correct comparison function based on the result. This requires us to migrate to a `struct hashmap`, as the khash interface does not give us the ability to change these functions. Note that we can unconditionally use `strihash()` to compute entry hashes regardless of case sensitivity: a hash function only needs to guarantee that equal keys have equal hashes, and a case-insensitive hash satisfies this requirement for both case-sensitive and case-insensitive equality. Overall it's quite debatable whether all of this complexity really is worth it, or whether we should just linearly search through all sources to find duplicates. But the mentioned commit cares about cases with thousands of alternates, and a linear search would of course regress performance quite a bit. This doesn't really feel like a reasonable case to care about though, but I don't feel comfortable regressing it anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 63 ++++++++++++++++++++++++++++++++++++++++-------------------- odb.h | 15 ++++++++++++++- odb/source.h | 7 +++++++ 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/odb.c b/odb.c index bd02d8ad54..51da386f22 100644 --- a/odb.c +++ b/odb.c @@ -2,11 +2,10 @@ #include "abspath.h" #include "commit-graph.h" #include "config.h" -#include "dir.h" #include "environment.h" #include "gettext.h" +#include "hashmap.h" #include "hex.h" -#include "khash.h" #include "lockfile.h" #include "loose.h" #include "midx.h" @@ -29,8 +28,32 @@ #include "trace2.h" #include "write-or-die.h" -KHASH_INIT(odb_path_map, const char * /* key: odb_path */, - struct odb_source *, 1, fspathhash, fspatheq) +static int odb_source_paths_cmp(struct object_database *o, + const char *a, const char *b) +{ + if (o->source_paths_icase < 0) { + int icase = 0; + repo_config_get_bool(o->repo, "core.ignorecase", &icase); + o->source_paths_icase = icase; + } + + return o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b); +} + +static int odb_source_by_path_cmp(const void *cb_data, + const struct hashmap_entry *entry, + const struct hashmap_entry *entry_or_key, + const void *keydata) +{ + struct object_database *o = (struct object_database *)cb_data; + const struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry); + const char *path = keydata; + + if (!path) + path = container_of(entry_or_key, const struct odb_source, by_path_entry)->path; + + return odb_source_paths_cmp(o, source->path, path); +} int odb_mkstemp(struct object_database *odb, struct strbuf *temp_filename, const char *pattern) @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb, */ static bool odb_is_source_usable(struct object_database *o, const char *path) { - int r; struct strbuf normalized_objdir = STRBUF_INIT; + struct hashmap_entry key; bool usable = false; strbuf_realpath(&normalized_objdir, o->sources->path, 1); @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path) * Prevent the common mistake of listing the same * thing twice, or object directory itself. */ - if (!o->source_by_path) { - khiter_t p; - - o->source_by_path = kh_init_odb_path_map(); + if (!hashmap_get_size(&o->source_by_path)) { assert(!o->sources->next); - p = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r); - assert(r == 1); /* never used */ - kh_value(o->source_by_path, p) = o->sources; + hashmap_entry_init(&o->sources->by_path_entry, + strihash(o->sources->path)); + hashmap_add(&o->source_by_path, &o->sources->by_path_entry); } - if (fspatheq(path, normalized_objdir.buf)) + if (!odb_source_paths_cmp(o, path, normalized_objdir.buf)) goto out; - if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path)) + hashmap_entry_init(&key, strihash(path)); + if (hashmap_get(&o->source_by_path, &key, path)) goto out; usable = true; @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * { struct odb_source *alternate = NULL; struct strvec sources = STRVEC_INIT; - khiter_t pos; - int ret; if (!odb_is_source_usable(odb, source)) goto error; @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * *odb->sources_tail = alternate; odb->sources_tail = &(alternate->next); - pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret); - if (!ret) + hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path)); + if (hashmap_get(&odb->source_by_path, &alternate->by_path_entry, + alternate->path)) BUG("source must not yet exist"); - kh_value(odb->source_by_path, pos) = alternate; + hashmap_add(&odb->source_by_path, &alternate->by_path_entry); /* recursively add alternates */ odb_source_read_alternates(alternate, &sources); @@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo, o->repo = repo; pthread_mutex_init(&o->replace_mutex, NULL); string_list_init_dup(&o->submodule_source_paths); + hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0); + o->source_paths_icase = -1; if (flags & ODB_NEW_HONOR_ENV) { primary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT)); @@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o) odb_source_free(o->inmemory_objects); o->inmemory_objects = NULL; - kh_destroy_odb_path_map(o->source_by_path); - o->source_by_path = NULL; + hashmap_clear(&o->source_by_path); } void odb_free(struct object_database *o) diff --git a/odb.h b/odb.h index 8eb4e85d64..71af7450a9 100644 --- a/odb.h +++ b/odb.h @@ -1,6 +1,7 @@ #ifndef ODB_H #define ODB_H +#include "hashmap.h" #include "object.h" #include "oidset.h" #include "oidmap.h" @@ -54,7 +55,19 @@ struct object_database { */ struct odb_source *sources; struct odb_source **sources_tail; - struct kh_odb_path_map *source_by_path; + + /* + * Map of object database sources, keyed by their respective paths. + * This map is used to detect the case where the same source is + * registered multiple times. + */ + struct hashmap source_by_path; + + /* + * Whether source paths shall be compared case-insensitively, as + * determined by "core.ignoreCase". + */ + int source_paths_icase; int loaded_alternates; diff --git a/odb/source.h b/odb/source.h index 4bc037b8d6..82cda8ad75 100644 --- a/odb/source.h +++ b/odb/source.h @@ -1,6 +1,7 @@ #ifndef ODB_SOURCE_H #define ODB_SOURCE_H +#include "hashmap.h" #include "object.h" #include "odb.h" #include "odb/transaction.h" @@ -50,6 +51,12 @@ struct strvec; struct odb_source { struct odb_source *next; + /* + * Entry in the object database's map of sources, keyed by this + * source's path. + */ + struct hashmap_entry by_path_entry; + /* Object database that owns this object source. */ struct object_database *odb; -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-12 9:13 ` [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt @ 2026-08-13 12:23 ` Karthik Nayak 2026-08-13 13:17 ` Patrick Steinhardt 0 siblings, 1 reply; 23+ messages in thread From: Karthik Nayak @ 2026-08-13 12:23 UTC (permalink / raw) To: Patrick Steinhardt, git; +Cc: Justin Tobler [-- Attachment #1: Type: text/plain, Size: 8660 bytes --] Patrick Steinhardt <ps@pks.im> writes: > When registering alternates we deduplicate object database sources by > their path so that the same source won't be added twice. Ever since > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07) > this duplicate check is backed by a map keyed by the source's path, > using `fspathhash()` and `fspatheq()` as hash and equality functions, > respectively. > > These functions are problematic in this context for two reasons: > > - They implicitly depend on `the_repository` instead of the > repository that owns the object database. > > - They derive case-sensitivity from `repo_ignore_case()`, which > returns a default value in case the repository's configuration has > not been parsed yet. Object database sources may be registered > before that is the case, so the answer may flip depending on when a > source gets registered. > > Fix this by making the comparison self-contained in the object > database. Instead of using `fspathhash()` and `fspatheq()` we resolve > "core.ignoreCase" manually and then use the correct comparison function > based on the result. This requires us to migrate to a `struct hashmap`, > as the khash interface does not give us the ability to change these > functions. > > Note that we can unconditionally use `strihash()` to compute entry > hashes regardless of case sensitivity: a hash function only needs to > guarantee that equal keys have equal hashes, and a case-insensitive > hash satisfies this requirement for both case-sensitive and > case-insensitive equality. > > Overall it's quite debatable whether all of this complexity really is > worth it, or whether we should just linearly search through all sources > to find duplicates. But the mentioned commit cares about cases with > thousands of alternates, and a linear search would of course regress > performance quite a bit. This doesn't really feel like a reasonable case > to care about though, but I don't feel comfortable regressing it anyway. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > odb.c | 63 ++++++++++++++++++++++++++++++++++++++++-------------------- > odb.h | 15 ++++++++++++++- > odb/source.h | 7 +++++++ > 3 files changed, 63 insertions(+), 22 deletions(-) > > diff --git a/odb.c b/odb.c > index bd02d8ad54..51da386f22 100644 > --- a/odb.c > +++ b/odb.c > @@ -2,11 +2,10 @@ > #include "abspath.h" > #include "commit-graph.h" > #include "config.h" > -#include "dir.h" > #include "environment.h" > #include "gettext.h" > +#include "hashmap.h" > #include "hex.h" > -#include "khash.h" > #include "lockfile.h" > #include "loose.h" > #include "midx.h" > @@ -29,8 +28,32 @@ > #include "trace2.h" > #include "write-or-die.h" > > -KHASH_INIT(odb_path_map, const char * /* key: odb_path */, > - struct odb_source *, 1, fspathhash, fspatheq) > +static int odb_source_paths_cmp(struct object_database *o, > + const char *a, const char *b) > +{ > + if (o->source_paths_icase < 0) { > + int icase = 0; > + repo_config_get_bool(o->repo, "core.ignorecase", &icase); > + o->source_paths_icase = icase; > + } > + Nit: couldn't this be simplified to if (o->source_paths_icase < 0) repo_config_get_bool(o->repo, "core.ignorecase", &o->source_paths_icase); > + return o->source_paths_icase ? strcasecmp(a, b) : strcmp(a, b); > +} > + > +static int odb_source_by_path_cmp(const void *cb_data, > + const struct hashmap_entry *entry, > + const struct hashmap_entry *entry_or_key, > + const void *keydata) > +{ > + struct object_database *o = (struct object_database *)cb_data; > + const struct odb_source *source = container_of(entry, const struct odb_source, by_path_entry); > + const char *path = keydata; > + > + if (!path) > + path = container_of(entry_or_key, const struct odb_source, by_path_entry)->path; > + > + return odb_source_paths_cmp(o, source->path, path); > +} > > int odb_mkstemp(struct object_database *odb, > struct strbuf *temp_filename, const char *pattern) > @@ -58,8 +81,8 @@ int odb_mkstemp(struct object_database *odb, > */ > static bool odb_is_source_usable(struct object_database *o, const char *path) > { > - int r; > struct strbuf normalized_objdir = STRBUF_INIT; > + struct hashmap_entry key; > bool usable = false; > > strbuf_realpath(&normalized_objdir, o->sources->path, 1); > @@ -76,20 +99,18 @@ static bool odb_is_source_usable(struct object_database *o, const char *path) > * Prevent the common mistake of listing the same > * thing twice, or object directory itself. > */ > - if (!o->source_by_path) { > - khiter_t p; > - > - o->source_by_path = kh_init_odb_path_map(); > + if (!hashmap_get_size(&o->source_by_path)) { > assert(!o->sources->next); > - p = kh_put_odb_path_map(o->source_by_path, o->sources->path, &r); > - assert(r == 1); /* never used */ > - kh_value(o->source_by_path, p) = o->sources; > + hashmap_entry_init(&o->sources->by_path_entry, > + strihash(o->sources->path)); > + hashmap_add(&o->source_by_path, &o->sources->by_path_entry); > } > > - if (fspatheq(path, normalized_objdir.buf)) > + if (!odb_source_paths_cmp(o, path, normalized_objdir.buf)) > goto out; > > - if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path)) > + hashmap_entry_init(&key, strihash(path)); > + if (hashmap_get(&o->source_by_path, &key, path)) > goto out; > > usable = true; > @@ -172,8 +193,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * > { > struct odb_source *alternate = NULL; > struct strvec sources = STRVEC_INIT; > - khiter_t pos; > - int ret; > > if (!odb_is_source_usable(odb, source)) > goto error; > @@ -184,10 +203,11 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database * > *odb->sources_tail = alternate; > odb->sources_tail = &(alternate->next); > > - pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret); > - if (!ret) > + hashmap_entry_init(&alternate->by_path_entry, strihash(alternate->path)); > + if (hashmap_get(&odb->source_by_path, &alternate->by_path_entry, > + alternate->path)) > BUG("source must not yet exist"); > - kh_value(odb->source_by_path, pos) = alternate; > + hashmap_add(&odb->source_by_path, &alternate->by_path_entry); > > /* recursively add alternates */ > odb_source_read_alternates(alternate, &sources); > @@ -1056,6 +1076,8 @@ struct object_database *odb_new(struct repository *repo, > o->repo = repo; > pthread_mutex_init(&o->replace_mutex, NULL); > string_list_init_dup(&o->submodule_source_paths); > + hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0); > + o->source_paths_icase = -1; > > if (flags & ODB_NEW_HONOR_ENV) { > primary_source = xstrdup_or_null(getenv(DB_ENVIRONMENT)); > @@ -1094,8 +1116,7 @@ static void odb_free_sources(struct object_database *o) > odb_source_free(o->inmemory_objects); > o->inmemory_objects = NULL; > > - kh_destroy_odb_path_map(o->source_by_path); > - o->source_by_path = NULL; > + hashmap_clear(&o->source_by_path); > } > > void odb_free(struct object_database *o) > diff --git a/odb.h b/odb.h > index 8eb4e85d64..71af7450a9 100644 > --- a/odb.h > +++ b/odb.h > @@ -1,6 +1,7 @@ > #ifndef ODB_H > #define ODB_H > > +#include "hashmap.h" > #include "object.h" > #include "oidset.h" > #include "oidmap.h" > @@ -54,7 +55,19 @@ struct object_database { > */ > struct odb_source *sources; > struct odb_source **sources_tail; > - struct kh_odb_path_map *source_by_path; > + > + /* > + * Map of object database sources, keyed by their respective paths. > + * This map is used to detect the case where the same source is > + * registered multiple times. > + */ > + struct hashmap source_by_path; > + > + /* > + * Whether source paths shall be compared case-insensitively, as > + * determined by "core.ignoreCase". > + */ > + int source_paths_icase; > > int loaded_alternates; > > diff --git a/odb/source.h b/odb/source.h > index 4bc037b8d6..82cda8ad75 100644 > --- a/odb/source.h > +++ b/odb/source.h > @@ -1,6 +1,7 @@ > #ifndef ODB_SOURCE_H > #define ODB_SOURCE_H > > +#include "hashmap.h" > #include "object.h" > #include "odb.h" > #include "odb/transaction.h" > @@ -50,6 +51,12 @@ struct strvec; > struct odb_source { > struct odb_source *next; > > + /* > + * Entry in the object database's map of sources, keyed by this > + * source's path. > + */ > + struct hashmap_entry by_path_entry; > + > /* Object database that owns this object source. */ > struct object_database *odb; > > > -- > 2.55.0.679.g6767b8d81c.dirty Apart from the nit, this patch looks good. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 690 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` 2026-08-13 12:23 ` Karthik Nayak @ 2026-08-13 13:17 ` Patrick Steinhardt 0 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-13 13:17 UTC (permalink / raw) To: Karthik Nayak; +Cc: git, Justin Tobler On Thu, Aug 13, 2026 at 05:23:39AM -0700, Karthik Nayak wrote: > Patrick Steinhardt <ps@pks.im> writes: > > diff --git a/odb.c b/odb.c > > index bd02d8ad54..51da386f22 100644 > > --- a/odb.c > > +++ b/odb.c > > @@ -29,8 +28,32 @@ > > #include "trace2.h" > > #include "write-or-die.h" > > > > -KHASH_INIT(odb_path_map, const char * /* key: odb_path */, > > - struct odb_source *, 1, fspathhash, fspatheq) > > +static int odb_source_paths_cmp(struct object_database *o, > > + const char *a, const char *b) > > +{ > > + if (o->source_paths_icase < 0) { > > + int icase = 0; > > + repo_config_get_bool(o->repo, "core.ignorecase", &icase); > > + o->source_paths_icase = icase; > > + } > > + > > Nit: couldn't this be simplified to > > if (o->source_paths_icase < 0) > repo_config_get_bool(o->repo, "core.ignorecase", &o->source_paths_icase); Not quite, as that wouldn't handle the case where the configuration isn't set. So we'd retain it as -1 and do the config lookup every single time. We could rewrite like this: if (o->source_paths_icase < 0 && repo_config_get_bool(o->repo, "core.ignorecase", &icase)) o->source_paths_icase = 0; But I'd argue that this is harder to read. Patrick ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/4] odb: eagerly initialize alternates 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt @ 2026-08-12 9:13 ` Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt ` (3 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 9:13 UTC (permalink / raw) To: git; +Cc: Justin Tobler When creating the object database we initialize the main object database source, but we don't yet initialize its alternates. Instead, we have many calls to `odb_prepare_alternates()` cluttered around the code base whenever we are about to iterate through the sources. This lazy loading doesn't really add much value: the moment where we read any object we _have_ to load the alternates anyway. So given that most of our commands would access the object database this optimization is not really buying us much in the first place. Quite on the contrary, it makes the code harder to understand and is a potential source of bugs in case any callsite forgot to prepare alternates before we iterate through the sources. Historically though there was a reason why we deferred lazy-loading: it may happen that the repository has "core.ignoreCase" configured, and we use that to deduplicate the list of alternates in case we had the same alternate configured multiple times, but with different casing. We used to initialize the object database before we had fully configured the owning repository though, and consequently we couldn't access that configuration yet. This has changed in the preceding commit though where we started to parse "core.ignoreCase" manually. Eagerly prepare alternates both when creating the object database and when flushing its caches. Drop the now-unneeded calls to prepare the alternates that are scattered across the code base. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/fsck.c | 3 --- builtin/pack-objects.c | 3 --- commit-graph.c | 4 ---- loose.c | 1 - object-name.c | 1 - odb.c | 26 ++++---------------------- odb.h | 6 ------ odb/streaming.c | 1 - pack-bitmap.c | 2 -- packfile.c | 1 - packfile.h | 2 -- 11 files changed, 4 insertions(+), 46 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index a6c054e45b..892c5661d9 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -1069,7 +1069,6 @@ int cmd_fsck(int argc, odb_for_each_object(repo->objects, NULL, mark_object_for_connectivity, repo, 0); } else { - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) fsck_source(repo, source); @@ -1155,7 +1154,6 @@ int cmd_fsck(int argc, if (repo->settings.core_commit_graph) { struct child_process commit_graph_verify = CHILD_PROCESS_INIT; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { child_process_init(&commit_graph_verify); commit_graph_verify.git_cmd = 1; @@ -1173,7 +1171,6 @@ int cmd_fsck(int argc, if (repo->settings.core_multi_pack_index) { struct child_process midx_verify = CHILD_PROCESS_INIT; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { child_process_init(&midx_verify); midx_verify.git_cmd = 1; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1ec5b6f206..48d37e8e32 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1779,8 +1779,6 @@ static int want_object_in_pack_mtime(const struct object_id *oid, *found_offset = 0; } - odb_prepare_alternates(the_repository->objects); - for (source = the_repository->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct multi_pack_index *m = get_multi_pack_index(files->packed); @@ -4520,7 +4518,6 @@ static void add_objects_in_unpacked_packs(void) .source_infop = &source_info, }; - odb_prepare_alternates(to_pack.repo->objects); for (source = to_pack.repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); diff --git a/commit-graph.c b/commit-graph.c index 49e8f63930..983c11ce85 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -651,8 +651,6 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, count = st->st_size / (odb->repo->hash_algo->hexsz + 1); CALLOC_ARRAY(oids, count); - odb_prepare_alternates(odb); - for (i = 0; i < count; i++) { struct odb_source *source; @@ -768,7 +766,6 @@ static struct commit_graph *prepare_commit_graph(struct repository *r) if (!commit_graph_compatible(r)) return NULL; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { r->objects->commit_graph = read_commit_graph_one(source); if (r->objects->commit_graph) @@ -2018,7 +2015,6 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx) _("Finding commits for commit graph among packed objects"), ctx->approx_nr_objects); - odb_prepare_alternates(ctx->r->objects); for (source = ctx->r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); odb_source_for_each_object(&files->packed->base, &oi, add_packed_commits_oi, diff --git a/loose.c b/loose.c index aa3cb1b4fc..c159d29d2d 100644 --- a/loose.c +++ b/loose.c @@ -115,7 +115,6 @@ int repo_read_loose_object_map(struct repository *repo) { struct odb_source *source; - odb_prepare_alternates(repo->objects); for (source = repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); if (loose_object_map_load(files->loose) < 0) diff --git a/object-name.c b/object-name.c index 83efba0ba6..34a08d76dd 100644 --- a/object-name.c +++ b/object-name.c @@ -280,7 +280,6 @@ static int init_object_disambiguation(struct repository *r, ds->len = len; ds->repo = r; - odb_prepare_alternates(r->objects); return 0; } diff --git a/odb.c b/odb.c index 51da386f22..2ae8228dd2 100644 --- a/odb.c +++ b/odb.c @@ -237,11 +237,6 @@ void odb_add_to_alternates_file(struct object_database *odb, struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, const char *dir) { - /* - * Make sure alternates are initialized, or else our entry may be - * overwritten when they are. - */ - odb_prepare_alternates(odb); return odb_add_alternate_recursively(odb, dir, 0); } @@ -250,12 +245,6 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, { struct odb_source *source; - /* - * Make sure alternates are initialized, or else our entry may be - * overwritten when they are. - */ - odb_prepare_alternates(odb); - /* * Make a new primary odb and link the old primary ODB in as an * alternate @@ -361,7 +350,6 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_ char *obj_dir_real = real_pathdup(obj_dir, 1); struct strbuf odb_path_real = STRBUF_INIT; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) { strbuf_realpath(&odb_path_real, source->path, 1); if (!strcmp(obj_dir_real, odb_path_real.buf)) @@ -495,7 +483,6 @@ int odb_for_each_alternate(struct object_database *odb, struct odb_source *alternate; int r = 0; - odb_prepare_alternates(odb); for (alternate = odb->sources->next; alternate; alternate = alternate->next) { r = cb(alternate, payload); if (r) @@ -504,7 +491,7 @@ int odb_for_each_alternate(struct object_database *odb, return r; } -void odb_prepare_alternates(struct object_database *odb) +static void odb_prepare_alternates(struct object_database *odb) { struct strvec sources = STRVEC_INIT; @@ -523,7 +510,6 @@ void odb_prepare_alternates(struct object_database *odb) int odb_has_alternates(struct object_database *odb) { - odb_prepare_alternates(odb); return !!odb->sources->next; } @@ -583,8 +569,6 @@ static int do_oid_object_info_extended(struct object_database *odb, if (!odb_source_read_object_info(odb->inmemory_objects, oid, oi, flags)) return 0; - odb_prepare_alternates(odb); - while (1) { struct odb_source *source; @@ -847,7 +831,6 @@ int odb_freshen_object(struct object_database *odb, const struct object_id *oid) { struct odb_source *source; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) if (odb_source_freshen_object(source, oid, NULL)) return 1; @@ -862,7 +845,6 @@ int odb_for_each_object_ext(struct object_database *odb, { int ret; - odb_prepare_alternates(odb); for (struct odb_source *source = odb->sources; source; source = source->next) { if (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local) continue; @@ -900,7 +882,6 @@ int odb_count_objects(struct object_database *odb, return 0; } - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) { unsigned long c; @@ -980,7 +961,6 @@ int odb_find_abbrev_len(struct object_database *odb, goto out; } - odb_prepare_alternates(odb); for (struct odb_source *source = odb->sources; source; source = source->next) { ret = odb_source_find_abbrev_len(source, oid, len, &len); if (ret) @@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo, o->alternate_db = secondary_sources; o->inmemory_objects = &odb_source_inmemory_new(o)->base; + odb_prepare_alternates(o); + free(primary_source); return o; } @@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) */ if (flags & ODB_PREPARE_FLUSH_CACHES) { o->loaded_alternates = 0; + odb_prepare_alternates(o); o->object_count_valid = 0; } - odb_prepare_alternates(o); for (source = o->sources; source; source = source->next) odb_source_prepare(source, flags); diff --git a/odb.h b/odb.h index 71af7450a9..fbafee174b 100644 --- a/odb.h +++ b/odb.h @@ -273,12 +273,6 @@ void odb_for_each_alternate_ref(struct object_database *odb, int odb_mkstemp(struct object_database *odb, struct strbuf *temp_filename, const char *pattern); -/* - * Prepare alternate object sources for the given database by reading - * "objects/info/alternates" and opening the respective sources. - */ -void odb_prepare_alternates(struct object_database *odb); - /* * Check whether the object database has any alternates. The primary object * source does not count as alternate. diff --git a/odb/streaming.c b/odb/streaming.c index 20531e864c..37642768e9 100644 --- a/odb/streaming.c +++ b/odb/streaming.c @@ -184,7 +184,6 @@ static int istream_source(struct odb_read_stream **out, { struct odb_source *source; - odb_prepare_alternates(odb); for (source = odb->sources; source; source = source->next) if (!odb_source_read_object_stream(out, source, oid)) return 0; diff --git a/pack-bitmap.c b/pack-bitmap.c index e85bd69ba4..e0fb57d332 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -717,7 +717,6 @@ static int open_bitmap(struct repository *r, assert(!bitmap_git->map); - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); @@ -3417,7 +3416,6 @@ int verify_bitmap_files(struct repository *r) struct packed_git *p; int res = 0; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct multi_pack_index *m = get_multi_pack_index(files->packed); diff --git a/packfile.c b/packfile.c index 0eee45055f..d870de90ed 100644 --- a/packfile.c +++ b/packfile.c @@ -1938,7 +1938,6 @@ int has_object_pack(struct repository *r, const struct object_id *oid) { struct odb_source *source; - odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); if (!odb_source_read_object_info(&files->packed->base, oid, NULL, 0)) diff --git a/packfile.h b/packfile.h index e1f77152b5..10de24f477 100644 --- a/packfile.h +++ b/packfile.h @@ -77,8 +77,6 @@ static inline struct repo_for_each_pack_data repo_for_eack_pack_data_init(struct { struct repo_for_each_pack_data data = { 0 }; - odb_prepare_alternates(repo->objects); - for (struct odb_source *source = repo->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); struct packfile_list_entry *entry = packfile_store_get_packs(files->packed); -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 3/4] odb: drop `loaded_alternates` field 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 2/4] odb: eagerly initialize alternates Patrick Steinhardt @ 2026-08-12 9:13 ` Patrick Steinhardt 2026-08-13 12:25 ` Karthik Nayak 2026-08-12 9:14 ` [PATCH v2 4/4] odb: drop `alternates_db` field Patrick Steinhardt ` (2 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 9:13 UTC (permalink / raw) To: git; +Cc: Justin Tobler The `struct object_database::loaded_alternates` field tells us whether or not alternates have been loaded already. This field was useful before the preceding commit as we were indeed lazy-loading alternates. But now that we started to eagerly load them we can assume them to be loaded after `odb_new()`, and hence the field does not serve any purpose anymore. Remove it. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 9 +-------- odb.h | 2 -- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/odb.c b/odb.c index 2ae8228dd2..2eb37a2f44 100644 --- a/odb.c +++ b/odb.c @@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb, int ret = odb_source_write_alternate(odb->sources, dir); if (ret < 0) die(NULL); - if (odb->loaded_alternates) - odb_add_alternate_recursively(odb, dir, 0); + odb_add_alternate_recursively(odb, dir, 0); } struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, @@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb) { struct strvec sources = STRVEC_INIT; - if (odb->loaded_alternates) - return; - parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources); odb_source_read_alternates(odb->sources, &sources); for (size_t i = 0; i < sources.nr; i++) odb_add_alternate_recursively(odb, sources.v[i], 0); - odb->loaded_alternates = 1; - strvec_clear(&sources); } @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) * the lifetime of the process. */ if (flags & ODB_PREPARE_FLUSH_CACHES) { - o->loaded_alternates = 0; odb_prepare_alternates(o); o->object_count_valid = 0; } diff --git a/odb.h b/odb.h index fbafee174b..aefb34213f 100644 --- a/odb.h +++ b/odb.h @@ -69,8 +69,6 @@ struct object_database { */ int source_paths_icase; - int loaded_alternates; - /* * A list of alternate object directories loaded from the environment; * this should not generally need to be accessed directly, but will -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/4] odb: drop `loaded_alternates` field 2026-08-12 9:13 ` [PATCH v2 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt @ 2026-08-13 12:25 ` Karthik Nayak 0 siblings, 0 replies; 23+ messages in thread From: Karthik Nayak @ 2026-08-13 12:25 UTC (permalink / raw) To: Patrick Steinhardt, git; +Cc: Justin Tobler [-- Attachment #1: Type: text/plain, Size: 2380 bytes --] Patrick Steinhardt <ps@pks.im> writes: > The `struct object_database::loaded_alternates` field tells us whether > or not alternates have been loaded already. This field was useful before > the preceding commit as we were indeed lazy-loading alternates. But now > that we started to eagerly load them we can assume them to be loaded > after `odb_new()`, and hence the field does not serve any purpose > anymore. > > Remove it. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > odb.c | 9 +-------- > odb.h | 2 -- > 2 files changed, 1 insertion(+), 10 deletions(-) > > diff --git a/odb.c b/odb.c > index 2ae8228dd2..2eb37a2f44 100644 > --- a/odb.c > +++ b/odb.c > @@ -230,8 +230,7 @@ void odb_add_to_alternates_file(struct object_database *odb, > int ret = odb_source_write_alternate(odb->sources, dir); > if (ret < 0) > die(NULL); > - if (odb->loaded_alternates) > - odb_add_alternate_recursively(odb, dir, 0); > + odb_add_alternate_recursively(odb, dir, 0); > } > > struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, > @@ -495,16 +494,11 @@ static void odb_prepare_alternates(struct object_database *odb) > { > struct strvec sources = STRVEC_INIT; > > - if (odb->loaded_alternates) > - return; > - > parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources); > odb_source_read_alternates(odb->sources, &sources); > for (size_t i = 0; i < sources.nr; i++) > odb_add_alternate_recursively(odb, sources.v[i], 0); > > - odb->loaded_alternates = 1; > - > strvec_clear(&sources); > } > > @@ -1132,7 +1126,6 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > * the lifetime of the process. > */ > if (flags & ODB_PREPARE_FLUSH_CACHES) { > - o->loaded_alternates = 0; > odb_prepare_alternates(o); > o->object_count_valid = 0; > } I was looking at this exact field in the previous commit and wondering if it needs to be removed, spot on. Makes sense. > diff --git a/odb.h b/odb.h > index fbafee174b..aefb34213f 100644 > --- a/odb.h > +++ b/odb.h > @@ -69,8 +69,6 @@ struct object_database { > */ > int source_paths_icase; > > - int loaded_alternates; > - > /* > * A list of alternate object directories loaded from the environment; > * this should not generally need to be accessed directly, but will > > -- > 2.55.0.679.g6767b8d81c.dirty The patch looks good. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 690 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/4] odb: drop `alternates_db` field 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt ` (2 preceding siblings ...) 2026-08-12 9:13 ` [PATCH v2 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt @ 2026-08-12 9:14 ` Patrick Steinhardt 2026-08-12 15:38 ` [PATCH v2 0/4] odb: eagerly load alternates Junio C Hamano 2026-08-13 12:28 ` Karthik Nayak 5 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-12 9:14 UTC (permalink / raw) To: git; +Cc: Justin Tobler The `struct object_database::alternates_db` field tracks the value of the "GIT_ALTERNATE_OBJECT_DIRECTORIES" environment variable and is used in `odb_prepare_alternates()`. It's not necessary to store it as a separate field anymore though, as we stopped lazy-loading alternates. Consequently, we can simply pass it to `odb_prepare_alternates()` via `odb_new()` now. Do so and remove the field. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- odb.c | 17 +++++++++-------- odb.h | 7 ------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/odb.c b/odb.c index 2eb37a2f44..0212eaa998 100644 --- a/odb.c +++ b/odb.c @@ -490,12 +490,14 @@ int odb_for_each_alternate(struct object_database *odb, return r; } -static void odb_prepare_alternates(struct object_database *odb) +static void odb_prepare_alternates(struct object_database *odb, + const char *alternate_db) { struct strvec sources = STRVEC_INIT; - parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources); + parse_alternates(alternate_db, PATH_SEP, NULL, &sources); odb_source_read_alternates(odb->sources, &sources); + for (size_t i = 0; i < sources.nr; i++) odb_add_alternate_recursively(odb, sources.v[i], 0); @@ -1062,11 +1064,11 @@ struct object_database *odb_new(struct repository *repo, o->sources = odb_source_new(o, primary_source, true); o->sources_tail = &o->sources->next; - o->alternate_db = secondary_sources; o->inmemory_objects = &odb_source_inmemory_new(o)->base; - odb_prepare_alternates(o); + odb_prepare_alternates(o, secondary_sources); + free(secondary_sources); free(primary_source); return o; } @@ -1100,8 +1102,6 @@ void odb_free(struct object_database *o) if (!o) return; - free(o->alternate_db); - oidmap_clear(&o->replace_map, 1); pthread_mutex_destroy(&o->replace_mutex); @@ -1123,10 +1123,11 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) * Reprepare alt odbs, in case the alternates file was modified * during the course of this process. This only _adds_ odbs to * the linked list, so existing odbs will continue to exist for - * the lifetime of the process. + * the lifetime of the process. Consequently, we don't have to + * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here. */ if (flags & ODB_PREPARE_FLUSH_CACHES) { - odb_prepare_alternates(o); + odb_prepare_alternates(o, NULL); o->object_count_valid = 0; } diff --git a/odb.h b/odb.h index aefb34213f..748366a610 100644 --- a/odb.h +++ b/odb.h @@ -69,13 +69,6 @@ struct object_database { */ int source_paths_icase; - /* - * A list of alternate object directories loaded from the environment; - * this should not generally need to be accessed directly, but will - * populate the "sources" list when odb_prepare_alternates() is run. - */ - char *alternate_db; - /* * Objects that should be substituted by other objects * (see git-replace(1)). -- 2.55.0.679.g6767b8d81c.dirty ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/4] odb: eagerly load alternates 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt ` (3 preceding siblings ...) 2026-08-12 9:14 ` [PATCH v2 4/4] odb: drop `alternates_db` field Patrick Steinhardt @ 2026-08-12 15:38 ` Junio C Hamano 2026-08-13 8:56 ` Patrick Steinhardt 2026-08-13 12:28 ` Karthik Nayak 5 siblings, 1 reply; 23+ messages in thread From: Junio C Hamano @ 2026-08-12 15:38 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Justin Tobler Patrick Steinhardt <ps@pks.im> writes: > The series is built on top of 010afd3166 (The 12th batch, 2026-08-07) > with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of > on-disk structures pluggable, 2026-08-07) merged into it. It is not a clean merge, though. Please double check the synthesized base when I push the integration results out later today. d296c52baa (Merge branch 'ps/odb-make-creation-pluggable' into ps/odb-eagerly-load-alternates, 2026-08-12) will be the merge, unless I notice and fix a mismerge in it before I push it out. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/4] odb: eagerly load alternates 2026-08-12 15:38 ` [PATCH v2 0/4] odb: eagerly load alternates Junio C Hamano @ 2026-08-13 8:56 ` Patrick Steinhardt 0 siblings, 0 replies; 23+ messages in thread From: Patrick Steinhardt @ 2026-08-13 8:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Justin Tobler On Wed, Aug 12, 2026 at 08:38:38AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > The series is built on top of 010afd3166 (The 12th batch, 2026-08-07) > > with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of > > on-disk structures pluggable, 2026-08-07) merged into it. > > It is not a clean merge, though. Please double check the > synthesized base when I push the integration results out later > today. d296c52baa (Merge branch 'ps/odb-make-creation-pluggable' > into ps/odb-eagerly-load-alternates, 2026-08-12) will be the merge, > unless I notice and fix a mismerge in it before I push it out. Hm. I'm probably missing something, but your merge is a bit curious as you merge the dependency into the feature branch instead of making it the base of it. If I do the following: # Switch to the master commit. $ git switch --detach 010afd3166 # Merge the dependnecy. $ git merge e927cfeb21 Then the only merge conflict I get is in "odb/source-files.c". This is a trivial merge conflict though, as it only impacts included headers. And then the rest of this series applies on top of that merge base without any further issues. Am I missing something? Thanks! Patrick ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/4] odb: eagerly load alternates 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt ` (4 preceding siblings ...) 2026-08-12 15:38 ` [PATCH v2 0/4] odb: eagerly load alternates Junio C Hamano @ 2026-08-13 12:28 ` Karthik Nayak 5 siblings, 0 replies; 23+ messages in thread From: Karthik Nayak @ 2026-08-13 12:28 UTC (permalink / raw) To: Patrick Steinhardt, git; +Cc: Justin Tobler [-- Attachment #1: Type: text/plain, Size: 5333 bytes --] Patrick Steinhardt <ps@pks.im> writes: > Hi, > > when initializing the object database we only eagerly initialize the > primary object database source. If the primary source has alternates, > those alternates are only initialized the first time we really access > the object database. > > When introduced in ace1534d6f (Introduce SHA1_FILE_DIRECTORIES to > support multiple object databases., 2005-05-07), alternates were > originally only loaded when a given object wasn't found in the primary > object database. This was also reinforced by later optimization, for > example in 693d2bc625 (Attempt to delay prepare_alt_odb during get_sha1, > 2007-05-26), where we tried to avoid loading alternates in even more > cases. But as Git has evolved, we eventually started to eagerly parse > alternates all over the codebase, including on every single object > lookup, and consequently deferring this operation does not really buy us > much anymore. > > The result of this is that we have calls to `odb_prepare_alternates()` > cluttered all over the code base. This is somewhat awkward, and as > almost every Git command ends up reading objects at it doesn't even buy > us anything. > > This patch series thus gets rid of the lazy-loading. Besides simplifying > the codebase a bit, it also prepares us for moving alternates into the > "files" backend as discussed in [1]. > > The series is built on top of 010afd3166 (The 12th batch, 2026-08-07) > with ps/odb-make-creation-pluggable at e927cfeb21 (odb: make creation of > on-disk structures pluggable, 2026-08-07) merged into it. > > Changes in v2: > - Add a missing word to a commit message. > - Explain why we don't have to handle GIT_ALTERNATE_OBJECT_DIRECTORIES > when re-preparing the object database. > - Link to v1: https://patch.msgid.link/20260810-pks-odb-eagerly-prepare-alternates-v1-0-f0fa4a4004e1@pks.im > V2 looks good, I have one nit, but it's not work re-rolling :) > Thanks! > > Patrick > > [1]: <amLgMqkqxR8mKIbT@pks.im> > > --- > Patrick Steinhardt (4): > odb: decouple source path comparisons from `the_repository` > odb: eagerly initialize alternates > odb: drop `loaded_alternates` field > odb: drop `alternates_db` field > > builtin/fsck.c | 3 -- > builtin/pack-objects.c | 3 -- > commit-graph.c | 4 -- > loose.c | 1 - > object-name.c | 1 - > odb.c | 109 ++++++++++++++++++++++++------------------------- > odb.h | 22 +++++----- > odb/source.h | 7 ++++ > odb/streaming.c | 1 - > pack-bitmap.c | 2 - > packfile.c | 1 - > packfile.h | 2 - > 12 files changed, 70 insertions(+), 86 deletions(-) > > Range-diff versus v1: > > 1: 25802adffa = 1: 721907c60d odb: decouple source path comparisons from `the_repository` > 2: 1e73b730d8 ! 2: 3b2c23566c odb: eagerly initialize alternates > @@ Commit message > many calls to `odb_prepare_alternates()` cluttered around the code base > whenever we are about to iterate through the sources. > > - This lazy loading doesn't really add much value: the moment where read > - any object we _have_ to load the alternates anyway. So given that most > - of our commands would access the object database this optimization is > - not really buying us much in the first place. Quite on the contrary, it > - makes the code harder to understand and is a potential source of bugs in > - case any callsite forgot to prepare alternates before we iterate through > - the sources. > + This lazy loading doesn't really add much value: the moment where we > + read any object we _have_ to load the alternates anyway. So given that > + most of our commands would access the object database this optimization > + is not really buying us much in the first place. Quite on the contrary, > + it makes the code harder to understand and is a potential source of bugs > + in case any callsite forgot to prepare alternates before we iterate > + through the sources. > > Historically though there was a reason why we deferred lazy-loading: it > may happen that the repository has "core.ignoreCase" configured, and we > 3: 2ca1aa2a37 = 3: df5d7df91d odb: drop `loaded_alternates` field > 4: 1e97c93bdf ! 4: 50a37ef385 odb: drop `alternates_db` field > @@ odb.c: void odb_free(struct object_database *o) > pthread_mutex_destroy(&o->replace_mutex); > > @@ odb.c: void odb_prepare(struct object_database *o, enum odb_prepare_flags flags) > - * the lifetime of the process. > + * Reprepare alt odbs, in case the alternates file was modified > + * during the course of this process. This only _adds_ odbs to > + * the linked list, so existing odbs will continue to exist for > +- * the lifetime of the process. > ++ * the lifetime of the process. Consequently, we don't have to > ++ * reprocess GIT_ALTERNATE_OBJECT_DIRECTORIES here. > */ > if (flags & ODB_PREPARE_FLUSH_CACHES) { > - odb_prepare_alternates(o); > > --- > base-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765 > change-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 690 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-08-13 13:17 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-10 13:33 [PATCH 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt 2026-08-11 22:04 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 2/4] odb: eagerly initialize alternates Patrick Steinhardt 2026-08-11 22:15 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 2026-08-10 13:33 ` [PATCH 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt 2026-08-11 22:22 ` Justin Tobler 2026-08-10 13:33 ` [PATCH 4/4] odb: drop `alternates_db` field Patrick Steinhardt 2026-08-11 22:31 ` Justin Tobler 2026-08-12 5:39 ` Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 0/4] odb: eagerly load alternates Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository` Patrick Steinhardt 2026-08-13 12:23 ` Karthik Nayak 2026-08-13 13:17 ` Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 2/4] odb: eagerly initialize alternates Patrick Steinhardt 2026-08-12 9:13 ` [PATCH v2 3/4] odb: drop `loaded_alternates` field Patrick Steinhardt 2026-08-13 12:25 ` Karthik Nayak 2026-08-12 9:14 ` [PATCH v2 4/4] odb: drop `alternates_db` field Patrick Steinhardt 2026-08-12 15:38 ` [PATCH v2 0/4] odb: eagerly load alternates Junio C Hamano 2026-08-13 8:56 ` Patrick Steinhardt 2026-08-13 12:28 ` Karthik Nayak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox