fix: don't sched_yield() on every object-lookup miss once all indices are loaded (#2878)

gix-odb/src/store_impls/dynamic/load_index.rs

@@ -5,7 +5,7 @@ use std::{
     path::{Path, PathBuf},
     sync::{
         Arc,
-        atomic::{AtomicU16, Ordering},
+        atomic::{AtomicUsize, Ordering},
     },
     time::SystemTime,
 };
@@ -123,14 +123,18 @@ impl super::Store {
         'retry_with_changed_index: loop {
             let previous_state_id = index.state_id();
             'retry_with_next_slot_index: loop {
+                // Announce the load *before* claiming a slot. Every claimed slot is then covered by an increment
+                // that happened before the claim, and the decrement only happens once loading is done (or failed).
+                // Hence a thread that finds nothing left to claim and observes this counter at zero afterwards knows
+                // that no claimed slot is still being loaded, without having to yield first to widen its window.
+                let ongoing_operation = IncOnNewAndDecOnDrop::new(&index.num_indices_currently_being_loaded);
                 match index
                     .next_index_to_load
                     .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |current| {
                         (current != index.slot_indices.len()).then_some(current + 1)
                     }) {
                     Ok(slot_map_index) => {
                         // This slot-map index is in bounds and was only given to us.
-                        let _ongoing_operation = IncOnNewAndDecOnDrop::new(&index.num_indices_currently_being_loaded);
                         let slot = &self.files[index.slot_indices[slot_map_index]];
                         let _lock = slot.write.lock();
                         if slot.generation.load(Ordering::SeqCst) > index.generation {
@@ -161,16 +165,14 @@ impl super::Store {
                         }
                     }
                     Err(_nothing_more_to_load) => {
+                        // We are not loading anything, so don't make anyone wait for us.
+                        drop(ongoing_operation);
                         // There can be contention as many threads start working at the same time and take all the
                         // slots to load indices for. Some threads might just be left-over and have to wait for something
                         // to change.
                         // TODO: potentially hot loop - could this be a condition variable?
-                        // This is a timing-based fix for the case that the `num_indices_being_loaded` isn't yet incremented,
-                        // and we might break out here without actually waiting for the loading operation. Then we'd fail to
-                        // observe a change and the underlying handler would not have all the indices it needs at its disposal.
-                        // Yielding means we will definitely loose enough time to observe the ongoing operation,
-                        // or its effects.
-                        std::thread::yield_now();
+                        // Once all indices are loaded, every miss ends up here with nothing to wait for, which must stay
+                        // syscall-free: a `contains()` miss is the hottest read path of this store.
                         while index.num_indices_currently_being_loaded.load(Ordering::SeqCst) != 0 {
                             std::thread::yield_now();
                         }
@@ -716,9 +718,9 @@ fn is_multipack_index(path: &Path) -> bool {
     path.file_name() == Some(OsStr::new("multi-pack-index"))
 }
 
-struct IncOnNewAndDecOnDrop<'a>(&'a AtomicU16);
+struct IncOnNewAndDecOnDrop<'a>(&'a AtomicUsize);
 impl<'a> IncOnNewAndDecOnDrop<'a> {
-    pub fn new(v: &'a AtomicU16) -> Self {
+    pub fn new(v: &'a AtomicUsize) -> Self {
         v.fetch_add(1, Ordering::SeqCst);
         Self(v)
     }

gix-odb/src/store_impls/dynamic/types.rs

@@ -2,7 +2,7 @@ use std::{
     path::{Path, PathBuf},
     sync::{
         Arc,
-        atomic::{AtomicU16, AtomicU32, AtomicUsize, Ordering},
+        atomic::{AtomicU32, AtomicUsize, Ordering},
     },
     time::SystemTime,
 };
@@ -104,7 +104,7 @@ pub struct SlotMapIndex {
     pub(crate) loaded_indices: Arc<AtomicUsize>,
     /// The amount of indices that are currently being loaded.
     /// Zero if no loading operation is currently happening, or more otherwise.
-    pub(crate) num_indices_currently_being_loaded: Arc<AtomicU16>,
+    pub(crate) num_indices_currently_being_loaded: Arc<AtomicUsize>,
 }
 
 impl SlotMapIndex {