[PATCH v3 0/5] odb: eagerly load alternates
Patrick Steinhardt <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <20260817-pks-odb-eagerly-prepare-alternates-v3-0-1115a7e02467@pks.im> |
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
---
base-commit: f6ad67a7977439ad8351d42e6ccfd11f714db765
change-id: 20260804-pks-odb-eagerly-prepare-alternates-3efb0a38e0dd