From 6e0cd6d38c5df874990ace6c2c3c0b39342c4d05 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 13 Dec 2021 10:33:29 +0800 Subject: [PATCH] Provide handle with a snapshot of the store's state (#266) --- experiments/odb-redesign/src/lib.rs | 40 ++++++++----- git-odb/src/lib.rs | 3 +- git-odb/src/store/general/find.rs | 7 +-- git-odb/src/store/general/handle.rs | 19 ++++-- git-odb/src/store/general/init.rs | 19 +++--- git-odb/src/store/general/load_indices.rs | 71 ++++++++++++----------- git-odb/src/store/general/metrics.rs | 4 +- git-odb/src/store/general/mod.rs | 6 +- git-odb/src/store/general/store.rs | 18 ++++-- git-traverse/tests/commit/mod.rs | 3 +- src/plumbing/main.rs | 6 +- src/porcelain/main.rs | 5 +- src/shared.rs | 3 +- 13 files changed, 123 insertions(+), 81 deletions(-) diff --git a/experiments/odb-redesign/src/lib.rs b/experiments/odb-redesign/src/lib.rs index 05c2fa98612..6b19494f473 100644 --- a/experiments/odb-redesign/src/lib.rs +++ b/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, @@ -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; diff --git a/git-odb/src/lib.rs b/git-odb/src/lib.rs index 98a60fc712b..9024c48d4b8 100644 --- a/git-odb/src/lib.rs +++ b/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}; diff --git a/git-odb/src/store/general/find.rs b/git-odb/src/store/general/find.rs index 9b9e4d52c73..3c669ce269b 100644 --- a/git-odb/src/store/general/find.rs +++ b/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 crate::pack::Find for super::Handle where diff --git a/git-odb/src/store/general/handle.rs b/git-odb/src/store/general/handle.rs index fc7958d1793..abc4b04cac1 100644 --- a/git-odb/src/store/general/handle.rs +++ b/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(), } } } diff --git a/git-odb/src/store/general/init.rs b/git-odb/src/store/general/init.rs index 7941da2d3e8..7a9e62d12c7 100644 --- a/git-odb/src/store/general/init.rs +++ b/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) -> std::io::Result { diff --git a/git-odb/src/store/general/load_indices.rs b/git-odb/src/store/general/load_indices.rs index 960e1d64823..7503e7bf650 100644 --- a/git-odb/src/store/general/load_indices.rs +++ b/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, // should probably be SmallVec to get around most allocations - loose_dbs: Arc>, - 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, // should probably be SmallVec to get around most allocations - loose_dbs: Arc>, - 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, // should probably be SmallVec to get around most allocations + loose_dbs: Arc>, + 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, - ) -> std::io::Result { + ) -> std::io::Result> { 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,7 +51,7 @@ 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 { @@ -59,11 +59,11 @@ impl super::Store { } } 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 { + fn consolidate_with_disk_state(&self, seen: StateId) -> std::io::Result> { 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) } } } diff --git a/git-odb/src/store/general/metrics.rs b/git-odb/src/store/general/metrics.rs index b4880c443e9..72b67940f28 100644 --- a/git-odb/src/store/general/metrics.rs +++ b/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; diff --git a/git-odb/src/store/general/mod.rs b/git-odb/src/store/general/mod.rs index 63846aec8ca..0812e707000 100644 --- a/git-odb/src/store/general/mod.rs +++ b/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, + snapshot: load_indices::Snapshot, } pub struct Store { diff --git a/git-odb/src/store/general/store.rs b/git-odb/src/store/general/store.rs index 428676a9dde..9533c848f92 100644 --- a/git-odb/src/store/general/store.rs +++ b/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)] diff --git a/git-traverse/tests/commit/mod.rs b/git-traverse/tests/commit/mod.rs index 20fe4cf6948..00ace92ee94 100644 --- a/git-traverse/tests/commit/mod.rs +++ b/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], diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 2b3277773fa..3b13887273c 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -12,8 +12,10 @@ use clap::Parser; use gitoxide_core as core; use gitoxide_core::pack::verify; -use crate::plumbing::options::{Args, Subcommands}; -use crate::shared::pretty::prepare_and_run; +use crate::{ + plumbing::options::{Args, Subcommands}, + shared::pretty::prepare_and_run, +}; #[cfg(feature = "gitoxide-core-async-client")] pub mod async_util { diff --git a/src/porcelain/main.rs b/src/porcelain/main.rs index aae15ee17b1..b59e087eddf 100644 --- a/src/porcelain/main.rs +++ b/src/porcelain/main.rs @@ -7,7 +7,10 @@ use anyhow::Result; use clap::Parser; use gitoxide_core as core; -use crate::{porcelain::options::Args, porcelain::options::Subcommands, shared::pretty::prepare_and_run}; +use crate::{ + porcelain::options::{Args, Subcommands}, + shared::pretty::prepare_and_run, +}; pub fn main() -> Result<()> { let args: Args = Args::parse(); diff --git a/src/shared.rs b/src/shared.rs index 12a28c9f9ed..429ee8897f2 100644 --- a/src/shared.rs +++ b/src/shared.rs @@ -140,9 +140,10 @@ pub mod pretty { } #[cfg(feature = "prodash-render-tui")] (true, true) | (false, true) => { - use crate::shared; use std::io::Write; + use crate::shared; + enum Event { UiDone, ComputationDone(Result, Vec),