git/midx.c, branch v2.51.2

midx: remove now-unused linked list of multi-pack indices

2025-07-15T19:07:30Z

In the preceding commits we have migrated all users of the linked list of multi-pack indices to instead use those stored in the object database sources. Remove those now-unused pointers. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

midx: stop using linked list when closing MIDX

2025-07-15T19:07:29Z

When calling `close_midx()` we not only close the multi-pack index for one object source, but instead we iterate through the whole linked list of MIDXs to close all of them. This linked list is about to go away in favor of using the new per-source pointer to its respective MIDX. Refactor the function to iterate through sources instead. Note that after this patch, there's a couple of callsites left that continue to use `close_midx()` without iterating through all sources. These are all cases where we don't care about the MIDX from other sources though, so it's fine to keep them as-is. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

midx: start tracking per object database source

2025-07-15T19:07:28Z

Multi-pack indices are tracked via `struct multi_pack_index`. This data structure is stored as a linked list inside `struct object_database`, which is the global database that spans across all of the object sources. This layout causes two problems: - Object databases consist of multiple object sources (e.g. one source per alternate object directory), where each multi-pack index is specific to one of those sources. Regardless of that though, the MIDX is not tracked per source, but tracked globally for the whole object database. This creates a mismatch between the on-disk layout and how things are organized in the object database subsystems and makes some parts, like figuring out whether a source has an MIDX, quite awkward. - Multi-pack indices are an implementation detail of how efficient access for packfiles work. As such, they are neither relevant in the context of loose objects, nor in a potential future where we have pluggable backends. Refactor `prepare_multi_pack_index_one()` so that it works on a specific source, which allows us to easily store a pointer to the multi-pack index inside of it. For now, this pointer exists next to the existing linked list we have in the object database. Users will be adjusted in subsequent patches to instead use the per-source pointers. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

object-store: rename `object_directory` to `odb_source`

2025-07-01T21:46:34Z

The `object_directory` structure is used as an access point for a single object directory like ".git/objects". While the structure isn't yet fully self-contained, the intent is for it to eventually contain all information required to access objects in one specific location. While the name "object directory" is a good fit for now, this will change over time as we continue with the agenda to make pluggable object databases a thing. Eventually, objects may not be accessed via any kind of directory at all anymore, but they could instead be backed by any kind of durable storage mechanism. While it seems quite far-fetched for now, it is thinkable that eventually this might even be some form of a database, for example. As such, the current name of this structure will become worse over time as we evolve into the direction of pluggable ODBs. Immediate next steps will start to carve out proper self-contained object directories, which requires us to pass in these object directories as parameters. Based on our modern naming schema this means that those functions should then be named after their subsystem, which means that we would start to bake the current name into the codebase more and more. Let's preempt this by renaming the structure. There have been a couple alternatives that were discussed: - `odb_backend` was discarded because it led to the association that one object database has a single backend, but the model is that one alternate has one backend. Furthermore, "backend" is more about the actual backing implementation and less about the high-level concept. - `odb_alternate` was discarded because it is a bit of a stretch to also call the main object directory an "alternate". Instead, pick `odb_source` as the new name. It makes it sufficiently clear that there can be multiple sources and does not cause confusion when mixed with the already-existing "alternate" terminology. In the future, this change allows us to easily introduce for example a `odb_files_source` and other format-specific implementations. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

midx: stop repeatedly looking up nonexistent packfiles

2025-05-28T14:56:29Z

The multi-pack index acts as a cache across a set of packfiles so that we can quickly look up which of those packfiles contains a given object. As such, the multi-pack index naturally needs to be updated every time one of the packfiles goes away, or otherwise the multi-pack index has grown stale. A stale multi-pack index should be handled gracefully by Git though, and in fact it is: if the indexed pack cannot be found we simply ignore it and eventually we fall back to doing the object lookup by just iterating through all packs, even if those aren't indexed. But while this fallback works, it has one significant downside: we don't cache the fact that a pack has vanished. This leads to us repeatedly trying to look up the same pack only to realize that it (still) doesn't exist. This issue can be easily demonstrated by creating a repository with a stale multi-pack index and a couple of objects. We do so by creating a repository with two packfiles, both of which are indexed by the multi-pack index, and then repack those two packfiles. Note that we have to move the multi-pack-index before doing the final repack, as Git knows to delete it otherwise. $ git init repo $ cd repo/ $ git config set maintenance.auto false $ for i in $(seq 1000); do printf "%d-original" $i >file-$i; done $ git add . $ git commit -moriginal $ git repack -dl $ for i in $(seq 1000); do printf "%d-modified" $i >file-$i; done $ git commit -a -mmodified $ git repack -dl $ git multi-pack-index write $ mv .git/objects/pack/multi-pack-index . $ git repack -Adl $ mv multi-pack-index .git/objects/pack/ Commands that cause a lot of objects lookups will now repeatedly invoke `add_packed_git()`, which leads to three failed access(3p) calls as well as one failed stat(3p) call. The following strace for example is done for `git log --patch` in the above repository: % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 74.67 0.024693 1 18038 18031 access 25.33 0.008378 1 6045 6017 newfstatat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.033071 1 24083 24048 total Fix the issue by introducing a negative lookup cache for indexed packs. This cache works by simply storing an invalid pointer for a missing pack when `prepare_midx_pack()` fails to look up the pack. Most users of the `packs` array don't need to be adjusted, either, as they all know to call `prepare_midx_pack()` before accessing the array. With this change in place we can now see a significantly reduced number of syscalls: % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 73.58 0.000323 5 60 28 newfstatat 26.42 0.000116 5 23 16 access ------ ----------- ----------- --------- --------- ---------------- 100.00 0.000439 5 83 44 total Furthermore, this change also results in a speedup: Benchmark 1: git log --patch (revision = HEAD~) Time (mean ± σ): 50.4 ms ± 2.5 ms [User: 22.0 ms, System: 24.4 ms] Range (min … max): 45.4 ms … 54.9 ms 53 runs Benchmark 2: git log --patch (revision = HEAD) Time (mean ± σ): 12.7 ms ± 0.4 ms [User: 11.1 ms, System: 1.6 ms] Range (min … max): 12.4 ms … 15.0 ms 191 runs Summary git log --patch (revision = HEAD) ran 3.96 ± 0.22 times faster than git log --patch (revision = HEAD~) In the end, it should in theory never be necessary to have this negative lookup cache given that we know to update the multi-pack index together with repacks. But as the change is quite contained and as the speedup can be significant as demonstrated above, it does feel sensible to have the negative lookup cache regardless. Based-on-patch-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

object-file: move `git_open_cloexec()` to "compat/open.c"

2025-04-15T15:24:35Z

The `git_open_cloexec()` wrapper function provides the ability to open a file with `O_CLOEXEC` in a platform-agnostic way. This function is provided by "object-file.c" even though it is not specific to the object subsystem at all. Move the file into "compat/open.c". This file already exists before this commit, but has only been compiled conditionally depending on whether or not open(3p) may return EINTR. With this change we now unconditionally compile the object, but wrap `git_open_with_retry()` in an ifdef. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

csum-file: stop depending on `the_repository`

2025-03-10T20:16:18Z

There are multiple sites in "csum-file.c" where we use the global `the_repository` variable, either explicitly or implicitly by using `the_hash_algo`. Refactor the code to stop using `the_repository` by adapting functions to receive required data as parameters. Adapt callsites accordingly by either using `the_repository->hash_algo`, or by using a context-provided hash algorithm in case the subsystem already got rid of its dependency on `the_repository`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

progress: stop using `the_repository`

2024-12-18T18:44:30Z

Stop using `the_repository` in the "progress" subsystem by passing in a repository when initializing `struct progress`. Furthermore, store a pointer to the repository in that struct so that we can pass it to the trace2 API when logging information. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/build-sign-compare' into ps/the-repository

2024-12-18T18:43:16Z

* ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano