Provide handle with a snapshot of the store's state (#266)

experiments/odb-redesign/src/lib.rs

@@ -1,33 +1,41 @@
 #![allow(dead_code, unused_variables, unreachable_code)]
 
 mod odb {
-    use arc_swap::ArcSwap;
-    use std::path::PathBuf;
-    use std::sync::atomic::{AtomicUsize, Ordering};
-    use std::sync::Arc;
+    use std::{
+        path::PathBuf,
+        sync::{
+            atomic::{AtomicUsize, Ordering},
+            Arc,
+        },
+    };
 
+    use arc_swap::ArcSwap;
     use git_hash::oid;
     use git_odb::pack::{data::entry::Location, find::Entry};
 
     use crate::odb::store::{load_indices, SlotIndexMarker, SlotMapIndex};
 
     pub mod store {
-        use arc_swap::ArcSwap;
-        use std::ops::Deref;
-        use std::sync::atomic::{AtomicUsize, Ordering};
-        use std::sync::Arc;
         use std::{
             io,
+            ops::Deref,
             path::{Path, PathBuf},
+            sync::{
+                atomic::{AtomicUsize, Ordering},
+                Arc,
+            },
         };
 
-        use crate::odb::Store;
+        use arc_swap::ArcSwap;
         use git_hash::oid;
 
+        use crate::odb::Store;
+
         mod index_file {
-            use crate::odb::store;
             use std::sync::Arc;
 
+            use crate::odb::store;
+
             pub enum SingleOrMulti {
                 Single {
                     index: Arc<git_pack::index::File>,
@@ -342,9 +350,10 @@ mod odb {
         }
 
         pub mod load_indices {
-            use crate::odb::{store, store::SlotIndexMarker};
             use std::sync::Arc;
 
+            use crate::odb::{store, store::SlotIndexMarker};
+
             pub(crate) enum Outcome {
                 /// Drop all data and fully replace it with `indices`.
                 /// This happens if we have witnessed a generational change invalidating all of our ids and causing currently loaded
@@ -603,8 +612,10 @@ mod odb {
 }
 
 mod refs {
-    use std::path::{Path, PathBuf};
-    use std::sync::Arc;
+    use std::{
+        path::{Path, PathBuf},
+        sync::Arc,
+    };
 
     pub struct LooseStore {
         path: PathBuf,
@@ -669,9 +680,10 @@ mod refs {
 }
 
 mod repository {
-    use crate::{odb, refs};
     use std::sync::Arc;
 
+    use crate::{odb, refs};
+
     mod raw {
         use git_pack::Find;
 

git-odb/src/lib.rs

@@ -15,9 +15,10 @@
 //!   * This is the database closely resembling the object database in a git repository, and probably what most people would want to use.
 //! * [`linked::Store`]
 //!   * A database containing various [`compound::Stores`][compound::Store] as gathered from `alternates` files.
+use std::path::PathBuf;
+
 use git_features::threading::OwnShared;
 pub use git_pack as pack;
-use std::path::PathBuf;
 
 mod store;
 pub use store::{compound, general, handle, linked, loose, sink, Cache, RefreshMode, Sink};

git-odb/src/store/general/find.rs

@@ -1,9 +1,8 @@
+use std::ops::Deref;
+
 use git_hash::oid;
 use git_object::Data;
-use git_pack::cache::DecodeEntry;
-use git_pack::data::entry::Location;
-use git_pack::index::Entry;
-use std::ops::Deref;
+use git_pack::{cache::DecodeEntry, data::entry::Location, index::Entry};
 
 impl<S> crate::pack::Find for super::Handle<S>
 where

git-odb/src/store/general/handle.rs

@@ -1,8 +1,11 @@
-use crate::general::store;
+use std::{
+    ops::Deref,
+    sync::{atomic::Ordering, Arc},
+};
+
 use git_features::threading::OwnShared;
-use std::ops::Deref;
-use std::sync::atomic::Ordering;
-use std::sync::Arc;
+
+use crate::general::store;
 
 pub(crate) mod multi_index {
     // TODO: replace this one with an actual implementation of a multi-pack index.
@@ -33,10 +36,12 @@ pub struct IndexForObjectInPack {
 }
 
 pub(crate) mod index_lookup {
-    use crate::general::{handle, store};
-    use git_hash::oid;
     use std::sync::Arc;
 
+    use git_hash::oid;
+
+    use crate::general::{handle, store};
+
     impl handle::IndexLookup {
         /// See if the oid is contained in this index, and return its full id for lookup possibly alongside its data file if already
         /// loaded.
@@ -111,6 +116,7 @@ impl super::Store {
             store: self.clone(),
             refresh_mode,
             token: Some(token),
+            snapshot: self.collect_snapshot(),
         }
     }
 }
@@ -151,6 +157,7 @@ where
             store: self.store.clone(),
             refresh_mode: self.refresh_mode,
             token: self.store.register_handle().into(),
+            snapshot: self.store.collect_snapshot(),
         }
     }
 }

git-odb/src/store/general/init.rs

@@ -1,12 +1,17 @@
-use crate::general::store;
-use crate::general::store::{MutableIndexAndPack, SlotMapIndex};
+use std::{
+    iter::FromIterator,
+    ops::Deref,
+    path::PathBuf,
+    sync::{atomic::AtomicUsize, Arc},
+};
+
 use arc_swap::ArcSwap;
 use git_features::threading::OwnShared;
-use std::iter::FromIterator;
-use std::ops::Deref;
-use std::path::PathBuf;
-use std::sync::atomic::AtomicUsize;
-use std::sync::Arc;
+
+use crate::general::{
+    store,
+    store::{MutableIndexAndPack, SlotMapIndex},
+};
 
 impl super::Store {
     pub fn at(objects_dir: impl Into<PathBuf>) -> std::io::Result<Self> {

git-odb/src/store/general/load_indices.rs

@@ -1,48 +1,48 @@
-use crate::general::{handle, store};
-use std::path::PathBuf;
+use std::{
+    path::PathBuf,
+    sync::{atomic::Ordering, Arc},
+};
 
-use crate::general::store::StateId;
-use crate::RefreshMode;
-use std::sync::atomic::Ordering;
-use std::sync::Arc;
+use crate::{
+    general::{handle, store, store::StateId},
+    RefreshMode,
+};
 
 pub(crate) enum Outcome {
     /// Drop all data and fully replace it with `indices`.
     /// This happens if we have witnessed a generational change invalidating all of our ids and causing currently loaded
     /// indices and maps to be dropped.
-    Replace {
-        indices: Vec<handle::IndexLookup>, // should probably be SmallVec to get around most allocations
-        loose_dbs: Arc<Vec<crate::loose::Store>>,
-        marker: store::SlotIndexMarker, // use to show where the caller left off last time
-    },
+    Replace(Snapshot),
     /// Despite all values being full copies, indices are still compatible to what was before. This also means
     /// the caller can continue searching the added indices and loose-dbs.
     /// Or in other words, new indices were only added to the known list, and what was seen before is known not to have changed.
     /// Besides that, the full internal state can be replaced as with `Replace`.
-    ReplaceStable {
-        indices: Vec<handle::IndexLookup>, // should probably be SmallVec to get around most allocations
-        loose_dbs: Arc<Vec<crate::loose::Store>>,
-        marker: store::SlotIndexMarker, // use to show where the caller left off last time
-    },
-    /// No new indices to look at, caller should give up
-    NoMoreIndices,
+    ReplaceStable(Snapshot),
+}
+
+pub(crate) struct Snapshot {
+    indices: Vec<handle::IndexLookup>, // should probably be SmallVec to get around most allocations
+    loose_dbs: Arc<Vec<crate::loose::Store>>,
+    marker: store::SlotIndexMarker, // use to show where the caller left off last time
 }
 
 impl super::Store {
+    /// If `None` is returned, there is new indices and the caller should give up. This is a possibility even if it's allowed to refresh
+    /// as here might be no change to pick up.
     pub(crate) fn load_next_indices(
         &self,
         refresh_mode: RefreshMode,
         marker: Option<store::SlotIndexMarker>,
-    ) -> std::io::Result<Outcome> {
+    ) -> std::io::Result<Option<Outcome>> {
         let index = self.index.load();
         let state_id = index.state_id();
-        if index.loose_dbs.is_empty() {
+        if !index.is_initialized() {
             // TODO: figure out what kind of refreshes we need. This one loads in the initial slot map, but I think this cost is paid
             //       in full during instantiation.
             return self.consolidate_with_disk_state(state_id);
         }
 
-        Ok(match marker {
+        Ok(Some(match marker {
             Some(marker) => {
                 if marker.generation != index.generation {
                     self.collect_replace_outcome(false /*stable*/)
@@ -51,19 +51,19 @@ impl super::Store {
 
                     // …and if that didn't yield anything new consider refreshing our disk state.
                     match refresh_mode {
-                        RefreshMode::Never => Outcome::NoMoreIndices,
+                        RefreshMode::Never => return Ok(None),
                         RefreshMode::AfterAllIndicesLoaded => return self.consolidate_with_disk_state(state_id),
                     }
                 } else {
                     self.collect_replace_outcome(true /*stable*/)
                 }
             }
             None => self.collect_replace_outcome(false /*stable*/),
-        })
+        }))
     }
 
     /// refresh and possibly clear out our existing data structures, causing all pack ids to be invalidated.
-    fn consolidate_with_disk_state(&self, seen: StateId) -> std::io::Result<Outcome> {
+    fn consolidate_with_disk_state(&self, seen: StateId) -> std::io::Result<Option<Outcome>> {
         let objects_directory = self.path.lock();
         if seen != self.index.load().state_id() {
             todo!("return …")
@@ -85,7 +85,7 @@ impl super::Store {
         self.num_handles_stable.load(Ordering::SeqCst) == 0
     }
 
-    fn collect_replace_outcome(&self, is_stable: bool) -> Outcome {
+    pub(crate) fn collect_snapshot(&self) -> Snapshot {
         let index = self.index.load();
         let indices = index
             .slot_indices
@@ -106,18 +106,19 @@ impl super::Store {
             })
             .collect();
 
+        Snapshot {
+            indices,
+            loose_dbs: Arc::clone(&index.loose_dbs),
+            marker: index.marker(),
+        }
+    }
+
+    fn collect_replace_outcome(&self, is_stable: bool) -> Outcome {
+        let snapshot = self.collect_snapshot();
         if is_stable {
-            Outcome::ReplaceStable {
-                indices,
-                loose_dbs: Arc::clone(&index.loose_dbs),
-                marker: index.marker(),
-            }
+            Outcome::ReplaceStable(snapshot)
         } else {
-            Outcome::Replace {
-                indices,
-                loose_dbs: Arc::clone(&index.loose_dbs),
-                marker: index.marker(),
-            }
+            Outcome::Replace(snapshot)
         }
     }
 }

git-odb/src/store/general/metrics.rs

@@ -1,7 +1,7 @@
-use crate::general::store;
-use crate::general::store::IndexAndPacks;
 use std::sync::atomic::Ordering;
 
+use crate::general::{store, store::IndexAndPacks};
+
 impl super::Store {
     pub fn metrics(&self) -> store::Metrics {
         let mut open_packs = 0;

git-odb/src/store/general/mod.rs

@@ -1,9 +1,8 @@
 #![allow(missing_docs, unused, dead_code)]
 
+use std::{ops::Deref, path::PathBuf, sync::atomic::AtomicUsize};
+
 use arc_swap::ArcSwap;
-use std::ops::Deref;
-use std::path::PathBuf;
-use std::sync::atomic::AtomicUsize;
 
 /// This effectively acts like a handle but exists to be usable from the actual `crate::Handle` implementation which adds caches on top.
 /// Each store is quickly cloned and contains thread-local state for shared packs.
@@ -15,6 +14,7 @@ where
     pub refresh_mode: crate::RefreshMode,
 
     pub(crate) token: Option<handle::Mode>,
+    snapshot: load_indices::Snapshot,
 }
 
 pub struct Store {

git-odb/src/store/general/store.rs

@@ -1,9 +1,14 @@
+use std::{
+    ops::BitXor,
+    path::{Path, PathBuf},
+    sync::{
+        atomic::{AtomicUsize, Ordering},
+        Arc,
+    },
+};
+
 use arc_swap::ArcSwap;
 use git_features::hash;
-use std::ops::BitXor;
-use std::path::{Path, PathBuf};
-use std::sync::atomic::{AtomicUsize, Ordering};
-use std::sync::Arc;
 
 /// An id to refer to an index file or a multipack index file
 pub type IndexId = usize;
@@ -68,6 +73,11 @@ impl SlotMapIndex {
             state_id: self.state_id(),
         }
     }
+
+    /// Returns true if we already know at least one loose object db, a sign of being initialized
+    pub(crate) fn is_initialized(&self) -> bool {
+        !self.loose_dbs.is_empty()
+    }
 }
 
 #[derive(Clone)]

git-traverse/tests/commit/mod.rs

@@ -1,9 +1,10 @@
 mod ancestor {
-    use crate::hex_to_id;
     use git_hash::{oid, ObjectId};
     use git_odb::{linked::Store, pack::FindExt};
     use git_traverse::commit;
 
+    use crate::hex_to_id;
+
     struct TraversalAssertion<'a> {
         init_script: &'a str,
         tips: &'a [&'a str],