Skip to content

gix-odb: Handle::contains issues a sched_yield on every miss, including under RefreshMode::Never #2878

Description

Summary

A membership test that misses issues a sched_yield(2) syscall, on every call, including with RefreshMode::Never and nothing loading.

Where

Handle::contains walks every loaded index, then every loose database, then asks load_one_index whether there is more to load. In steady state — all indices loaded, no concurrent loading — that lands in load_next_index's Err(_nothing_more_to_load) arm:

// gix-odb/src/store_impls/dynamic/load_index.rs
Err(_nothing_more_to_load) => {
    // …
    // TODO: potentially hot loop - could this be a condition variable?
    // …
    std::thread::yield_now();
    while index.num_indices_currently_being_loaded.load(Ordering::SeqCst) != 0 {
        std::thread::yield_now();
    }
    break 'retry_with_next_slot_index;
}

The first yield_now() is unconditional — it runs before anything has been observed about num_indices_currently_being_loaded, and the comment above explains why: it is a timing fix for the window in which another thread has taken a slot but not yet incremented the counter.

Measured

A single-threaded process asking Handle::contains for 20 000 object ids that are not present, against a 12 328-object repository with one pack and no loose objects at all. strace -c -f:

syscall calls per miss
sched_yield 20 001 1.00
statx 20 046 (20 002 ENOENT) 1.00

The same binary asked 12 328 ids that are present issues neither. Both are miss-only.

Removing just the unconditional first yield_now() and keeping the wait loop — minimum of nine runs of 200 000 misses each, release build, x86-64:

arm ns per miss
stock 1045.1
first yield_now() removed 783.8

261 ns per miss, 25 % of the whole cost of a miss. For context, on the same box git's own .idx fanout + binary search answers the same question in ~302 ns, and everything gix-odb does apart from these syscalls costs 198 ns.

The question, not a patch

The wait loop is doing real work and should stay. The question is only about the yield_now() in front of it: once num_indices_currently_being_loaded has been observed zero, is the extra unconditional yield still buying anything, or could the arm be

while index.num_indices_currently_being_loaded.load(Ordering::SeqCst) != 0 {
    std::thread::yield_now();
}

with the widened observation window achieved some other way — the condition variable the TODO already proposes, or an acquire/release pairing with the increment?

I am deliberately not claiming the removal is safe. The comment describes a real slot-map race and whoever wrote it knows the protocol better than I do. What I am reporting is the cost: a syscall on the hottest read path gix-odb has, paid by every caller of contains that misses, whether or not any loading is happening.

Not filed, but noted while measuring

loose::Store::contains is hash_path(id, self.path.clone()).is_file() — one PathBuf heap clone and one statx per miss per loose database. That is correct (a loose object can appear at any time and there is no index to consult), and at 586 ns it is the largest single term in a miss, so I mention it only so the next person to profile this path finds it already named. If the allocation ever matters, hash_path could take a &Path and a reused scratch buffer without any behaviour change.

Environment

  • gix-odb 0.83.0, gix-pack 0.73.0, gix-hash 0.26.0
  • Linux 6.x, x86-64
  • reached through gix_odb::Cache<Handle> with refresh_never() and set_pack_cache, i.e. the configuration a read-only server holds

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions