Re: [PATCH v3 0/5] odb: eagerly load alternates

Karthik Nayak <[email protected]> Thu, 20 Aug 2026 05:11:07 -0400
Newsgroups	org.kernel.vger.git
Message-ID	<CAOLa=ZReodSXjEbQkFoxcofMLq6mUOjXANRg7bZ2uEKKQn=DXw@mail.gmail.com>
Patrick Steinhardt <[email protected]> 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 v3:
>   - Create object database after we have written the complete repository
>     configuration in `init_db()`.
>   - Document that we might want to drop case-insensitive deduplication
>     of alternates going forward.
>   - Better explain why we have to migrate to `struct hashmap`.
>   - Link to v2: https://patch.msgid.link/20260812-pks-odb-eagerly-prepare-alternates-v2-0-522b9a5bc1ea@pks.im
>
> 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]: <[email protected]>
>
> ---
> Patrick Steinhardt (5):
>       setup: create ref and object databases after config is written
>       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                  | 124 +++++++++++++++++++++++++++----------------------
>  odb.h                  |  22 ++++-----
>  odb/source.h           |   7 +++
>  odb/streaming.c        |   1 -
>  pack-bitmap.c          |   2 -
>  packfile.c             |   1 -
>  packfile.h             |   2 -
>  setup.c                |  12 ++---
>  13 files changed, 91 insertions(+), 92 deletions(-)
>
> Range-diff versus v2:
>
> -:  ---------- > 1:  2adb64d17c setup: create ref and object databases after config is written
> 1:  6255ac7964 ! 2:  736b8d8eb4 odb: decouple source path comparisons from `the_repository`
>     @@ Commit message
>          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.
>     +    as the khash interface does not give us the ability to pass an arbitrary
>     +    payload to these functions, and hence we'd have to use global state to
>     +    decide which of those to use.
>
>          Note that we can unconditionally use `strihash()` to compute entry
>          hashes regardless of case sensitivity: a hash function only needs to
>     @@ Commit message
>          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.
>     +    worth it, out of two reasons:
>     +
>     +      - We could 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, but I
>     +        don't feel comfortable regressing it anyway.
>     +
>     +      - It's dubious whether we should handle "core.ignoreCase" in the first
>     +        place. The downside would be that we might add the same alternate
>     +        multiple times with different casing. But this is an edge case, and
>     +        it's not even fully fixed because we don't resolve symlinks or
>     +        mountpoints, either.
>     +
>     +    So for now, keep this infrastructure in-place while removing the global
>     +    dependency on `the_repository`. We may want to revisit this in the
>     +    future though.
>
>          Signed-off-by: Patrick Steinhardt <[email protected]>
>
>     @@ odb.c
>
>      -KHASH_INIT(odb_path_map, const char * /* key: odb_path */,
>      -	struct odb_source *, 1, fspathhash, fspatheq)
>     ++/*
>     ++ * NEEDSWORK: we're using "core.ignoreCase" to deduplicate alternates that
>     ++ * _may_ be the same. This requires quite a bit of boilerplate for dubious
>     ++ * benefit:
>     ++ *
>     ++ *   - Duplicating alternates should really only lead to regressed performance.
>     ++ *
>     ++ *   - We don't properly resolve symlinks or mointpoints, so we may still end
>     ++ *     up duplicating alternates.
>     ++ *
>     ++ *   - The value may be lying, in which case we might deduplicate alternates
>     ++ *     that are in fact not mapping to the same directory.
>     ++ *
>     ++ * We should investigate whether we can remove this whole mechanism outright.
>     ++ */
>      +static int odb_source_paths_cmp(struct object_database *o,
>      +				const char *a, const char *b)
>      +{
> 2:  4743659d76 = 3:  a9db918b49 odb: eagerly initialize alternates
> 3:  4a62dde9d0 = 4:  369a566a6a odb: drop `loaded_alternates` field
> 4:  e978a5a47d = 5:  a0a22a0bd2 odb: drop `alternates_db` field
>

The new patch and the range-diff looks good. Thanks!
signature.asc (application/pgp-signature, 690 B)