Skip to content

Commit

Permalink
Default handle refresh mode is the least surprising, with option to c…
Browse files Browse the repository at this point in the history
…onfigure (#266)
  • Loading branch information
Byron committed Dec 19, 2021
1 parent c301abe commit 1b74c14
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 41 deletions.
5 changes: 2 additions & 3 deletions experiments/object-access/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{path::Path, sync::Arc, time::Instant};
use anyhow::anyhow;
use git_repository::{hash::ObjectId, odb, threading::OwnShared, Repository};

use crate::odb::{Cache, RefreshMode};
use crate::odb::Cache;

const GITOXIDE_STATIC_CACHE_SIZE: usize = 64;
const GITOXIDE_CACHED_OBJECT_DATA_PER_THREAD_IN_BYTES: usize = 60_000_000;
Expand Down Expand Up @@ -465,8 +465,7 @@ where
Some(store) => store,
None => OwnShared::new(odb::Store::at_opts(objects_dir, slots)?),
};
let handle =
Cache::from(store.to_handle(RefreshMode::AfterAllIndicesLoaded)).with_pack_cache(move || Box::new(new_cache()));
let handle = Cache::from(store.to_handle()).with_pack_cache(move || Box::new(new_cache()));

git_repository::parallel::in_parallel(
hashes.chunks(1000),
Expand Down
4 changes: 2 additions & 2 deletions git-odb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use git_features::zlib::stream::deflate;
pub use git_pack as pack;

mod store_impls;
pub use store_impls::{compound, dynamic as store, linked, loose, RefreshMode};
pub use store_impls::{compound, dynamic as store, linked, loose};

pub mod alternate;

Expand Down Expand Up @@ -109,7 +109,7 @@ impl Store {

/// Create a new cached odb handle with support for additional options.
pub fn at_opts(objects_dir: impl Into<PathBuf>, slots: store::init::Slots) -> std::io::Result<Handle> {
let handle = OwnShared::new(Store::at_opts(objects_dir, slots)?).to_handle(RefreshMode::AfterAllIndicesLoaded);
let handle = OwnShared::new(Store::at_opts(objects_dir, slots)?).to_handle();
Ok(Cache::from(handle))
}

Expand Down
21 changes: 11 additions & 10 deletions git-odb/src/store_impls/dynamic/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{

use git_features::threading::OwnShared;

use crate::store::{handle, types};
use crate::store::{handle, types, RefreshMode};

pub(crate) mod multi_index {
// TODO: replace this one with an actual implementation of a multi-pack index.
Expand Down Expand Up @@ -162,31 +162,28 @@ impl super::Store {
/// Handle creation
impl super::Store {
pub fn to_cache(self: &OwnShared<Self>) -> crate::Cache<super::Handle<OwnShared<super::Store>>> {
self.to_handle(crate::RefreshMode::AfterAllIndicesLoaded).into()
self.to_handle().into()
}

pub fn to_cache_arc(self: &Arc<Self>) -> crate::Cache<super::Handle<Arc<super::Store>>> {
self.to_handle_arc(crate::RefreshMode::AfterAllIndicesLoaded).into()
self.to_handle_arc().into()
}

pub fn to_handle(
self: &OwnShared<Self>,
refresh_mode: crate::RefreshMode,
) -> super::Handle<OwnShared<super::Store>> {
pub fn to_handle(self: &OwnShared<Self>) -> super::Handle<OwnShared<super::Store>> {
let token = self.register_handle();
super::Handle {
store: self.clone(),
refresh_mode,
refresh_mode: RefreshMode::default(),
token: Some(token),
snapshot: RefCell::new(self.collect_snapshot()),
}
}

pub fn to_handle_arc(self: &Arc<Self>, refresh_mode: crate::RefreshMode) -> super::Handle<Arc<super::Store>> {
pub fn to_handle_arc(self: &Arc<Self>) -> super::Handle<Arc<super::Store>> {
let token = self.register_handle();
super::Handle {
store: self.clone(),
refresh_mode,
refresh_mode: Default::default(),
token: Some(token),
snapshot: RefCell::new(self.collect_snapshot()),
}
Expand Down Expand Up @@ -223,6 +220,10 @@ where
pub fn store_owned(&self) -> S {
self.store.clone()
}

pub fn refresh_never(&mut self) {
self.refresh_mode = RefreshMode::Never;
}
}

impl<S> Drop for super::Handle<S>
Expand Down
3 changes: 2 additions & 1 deletion git-odb/src/store_impls/dynamic/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::{ops::Deref, option::Option::None, sync::Arc, vec::IntoIter};

use git_hash::ObjectId;

use crate::store::RefreshMode;
use crate::{loose, store::handle, store_impls::dynamic};

enum State {
Expand Down Expand Up @@ -29,7 +30,7 @@ impl AllObjects {
/// Create a new iterator from a general database, which will be forced to load all indices eagerly.
pub fn new(db: &dynamic::Store) -> Result<Self, crate::store::load_index::Error> {
let mut snapshot = db.collect_snapshot();
while let Some(new_snapshot) = db.load_one_index(crate::RefreshMode::Never, snapshot.marker)? {
while let Some(new_snapshot) = db.load_one_index(RefreshMode::Never, snapshot.marker)? {
snapshot = new_snapshot
}

Expand Down
5 changes: 1 addition & 4 deletions git-odb/src/store_impls/dynamic/load_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use std::{
time::SystemTime,
};

use crate::{
store::{handle, types},
RefreshMode,
};
use crate::store::{handle, types, RefreshMode};

pub(crate) struct Snapshot {
/// Indices ready for object lookup or contains checks, ordered usually by modification data, recent ones first.
Expand Down
20 changes: 19 additions & 1 deletion git-odb/src/store_impls/dynamic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,30 @@ where
S: Deref<Target = Store> + Clone,
{
pub(crate) store: S,
pub refresh_mode: crate::RefreshMode,
pub refresh_mode: RefreshMode,

pub(crate) token: Option<handle::Mode>,
snapshot: RefCell<load_index::Snapshot>,
}

/// Define how packs will be refreshed when all indices are loaded, which is useful if a lot of objects are missing.
#[derive(Clone, Copy)]
pub enum RefreshMode {
/// Check for new or changed pack indices (and pack data files) when the last known index is loaded.
/// During runtime we will keep pack indices stable by never reusing them, however, there is the option for
/// clearing internal caches which is likely to change pack ids and it will trigger unloading of packs as they are missing on disk.
AfterAllIndicesLoaded,
/// Use this if you expect a lot of missing objects that shouldn't trigger refreshes even after all packs are loaded.
/// This comes at the risk of not learning that the packs have changed in the mean time.
Never,
}

impl Default for RefreshMode {
fn default() -> Self {
RefreshMode::AfterAllIndicesLoaded
}
}

///
pub mod find;

Expand Down
12 changes: 0 additions & 12 deletions git-odb/src/store_impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,3 @@ pub mod compound;
pub mod dynamic;
pub mod linked;
pub mod loose;

/// Define how packs will be refreshed when all indices are loaded, which is useful if a lot of objects are missing.
#[derive(Clone, Copy)]
pub enum RefreshMode {
/// Check for new or changed pack indices (and pack data files) when the last known index is loaded.
/// During runtime we will keep pack indices stable by never reusing them, however, there is the option for
/// clearing internal caches which is likely to change pack ids and it will trigger unloading of packs as they are missing on disk.
AfterAllIndicesLoaded,
/// Use this if you expect a lot of missing objects that shouldn't trigger refreshes even after all packs are loaded.
/// This comes at the risk of not learning that the packs have changed in the mean time.
Never,
}
10 changes: 5 additions & 5 deletions git-odb/tests/odb/store/general.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(unused)]
use git_features::threading::OwnShared;
use git_odb::{store, Find, FindExt, RefreshMode, Write};
use git_odb::{store, Find, FindExt, Write};
use git_testtools::{fixture_path, hex_to_id};

fn db() -> git_odb::Handle {
Expand All @@ -12,7 +12,7 @@ fn write() -> crate::Result {
let dir = tempfile::tempdir()?;
let mut handle = git_odb::at(dir.path())?;
// It should refresh once even if the refresh mode is never, just to initialize the index
handle.inner.refresh_mode = git_odb::RefreshMode::Never;
handle.inner.refresh_mode = store::RefreshMode::Never;

let written_id = handle.write_buf(git_object::Kind::Blob, b"hello world", git_hash::Kind::Sha1)?;
assert_eq!(written_id, hex_to_id("95d09f2b10159347eece71399a7e2e907ea3df4f"));
Expand Down Expand Up @@ -93,7 +93,7 @@ fn contains() {
"trigger refreshes each time there is an object miss"
);

new_handle.inner.refresh_mode = RefreshMode::Never;
new_handle.inner.refresh_mode = store::RefreshMode::Never;
assert!(!new_handle.contains(hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")));
assert_eq!(
new_handle.inner.store().metrics(),
Expand Down Expand Up @@ -162,7 +162,7 @@ fn lookup() {

assert!(matches!(
handle.inner.refresh_mode,
git_odb::RefreshMode::AfterAllIndicesLoaded
store::RefreshMode::AfterAllIndicesLoaded
));
assert!(!handle.contains(hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")));

Expand All @@ -173,7 +173,7 @@ fn lookup() {
"it tried to refresh once to see if the missing object is there then"
);

handle.inner.refresh_mode = git_odb::RefreshMode::Never;
handle.inner.refresh_mode = store::RefreshMode::Never;
let previous_refresh_count = all_loaded.num_refreshes;
assert!(!handle.contains(hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")));
assert_eq!(
Expand Down
4 changes: 1 addition & 3 deletions git-repository/src/easy/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ impl From<&crate::Repository> for easy::Handle {
fn from(repo: &crate::Repository) -> Self {
easy::Handle::from_refs_and_objects(
repo.refs.clone(),
repo.objects
.to_handle(git_odb::RefreshMode::AfterAllIndicesLoaded)
.into(),
repo.objects.to_handle().into(),
repo.hash_kind,
repo.work_tree.clone(),
)
Expand Down

0 comments on commit 1b74c14

Please sign in to comment.