Skip to content

Commit

Permalink
Merge branch 'improvements'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 21, 2024
2 parents 7b7902e + f944e49 commit 242fedc
Show file tree
Hide file tree
Showing 47 changed files with 935 additions and 216 deletions.
2 changes: 1 addition & 1 deletion gitoxide-core/src/hours/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub fn spawn_tree_delta_threads<'scope>(
for chunk in rx {
for (commit_idx, parent_commit, commit) in chunk {
if let Some(cache) = cache.as_mut() {
cache.clear_resource_cache();
cache.clear_resource_cache_keep_allocation();
}
commits.fetch_add(1, Ordering::Relaxed);
if gix::interrupt::is_triggered() {
Expand Down
4 changes: 2 additions & 2 deletions gitoxide-core/src/query/engine/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ pub fn update(
Some(c) => c,
None => continue,
};
rewrite_cache.clear_resource_cache();
diff_cache.clear_resource_cache();
rewrite_cache.clear_resource_cache_keep_allocation();
diff_cache.clear_resource_cache_keep_allocation();
from.changes()?
.track_path()
.track_rewrites(Some(rewrites))
Expand Down
2 changes: 2 additions & 0 deletions gix-diff/src/blob/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ pub struct Platform {
/// That way, expensive rewrite-checks with NxM matrix checks would be as fast as possible,
/// avoiding duplicate work.
diff_cache: HashMap<platform::CacheKey, platform::CacheValue>,
/// A list of previously used buffers, ready for re-use.
free_list: Vec<Vec<u8>>,
}

mod impls {
Expand Down
40 changes: 36 additions & 4 deletions gix-diff/src/blob/platform.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{io::Write, process::Stdio};

use bstr::{BStr, BString, ByteSlice};
use std::cmp::Ordering;
use std::{io::Write, process::Stdio};

use super::Algorithm;
use crate::blob::{pipeline, Pipeline, Platform, ResourceKind};
Expand Down Expand Up @@ -325,6 +325,7 @@ impl Platform {
old: None,
new: None,
diff_cache: Default::default(),
free_list: Vec::with_capacity(2),
options,
filter,
filter_mode,
Expand Down Expand Up @@ -542,7 +543,7 @@ impl Platform {

/// Every call to [set_resource()](Self::set_resource()) will keep the diffable data in memory, and that will never be cleared.
///
/// Use this method to clear the cache, releasing memory. Note that this will also loose all information about resources
/// Use this method to clear the cache, releasing memory. Note that this will also lose all information about resources
/// which means diffs would fail unless the resources are set again.
///
/// Note that this also has to be called if the same resource is going to be diffed in different states, i.e. using different
Expand All @@ -551,6 +552,37 @@ impl Platform {
self.old = None;
self.new = None;
self.diff_cache.clear();
self.free_list.clear();
}

/// Every call to [set_resource()](Self::set_resource()) will keep the diffable data in memory, and that will never be cleared.
///
/// Use this method to clear the cache, but keep the previously used buffers around for later re-use.
///
/// If there are more buffers on the free-list than there are stored sources, we half that amount each time this method is called,
/// or keep as many resources as were previously stored, or 2 buffers, whatever is larger.
/// If there are fewer buffers in the free-list than are in the resource cache, we will keep as many as needed to match the
/// number of previously stored resources.
///
/// Returns the number of available buffers.
pub fn clear_resource_cache_keep_allocation(&mut self) -> usize {
self.old = None;
self.new = None;

let diff_cache = std::mem::take(&mut self.diff_cache);
match self.free_list.len().cmp(&diff_cache.len()) {
Ordering::Less => {
let to_take = diff_cache.len() - self.free_list.len();
self.free_list
.extend(diff_cache.into_values().map(|v| v.buffer).take(to_take));
}
Ordering::Equal => {}
Ordering::Greater => {
let new_len = (self.free_list.len() / 2).max(diff_cache.len()).max(2);
self.free_list.truncate(new_len);
}
}
self.free_list.len()
}
}

Expand Down Expand Up @@ -591,7 +623,7 @@ impl Platform {
kind,
rela_path: rela_path.to_owned(),
})?;
let mut buf = Vec::new();
let mut buf = self.free_list.pop().unwrap_or_default();
let out = self.filter.convert_to_diffable(
&id,
mode,
Expand Down
24 changes: 23 additions & 1 deletion gix-diff/tests/blob/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,35 @@ fn resources_of_worktree_and_odb_and_check_link() -> crate::Result {
"Also obvious that symlinks are definitely special, but it's what git does as well"
);

platform.clear_resource_cache();
assert_eq!(
platform.clear_resource_cache_keep_allocation(),
3,
"some buffers are retained and reused"
);
assert_eq!(
platform.resources(),
None,
"clearing the cache voids resources and one has to set it up again"
);

assert_eq!(
platform.clear_resource_cache_keep_allocation(),
2,
"doing this again keeps 2 buffers"
);
assert_eq!(
platform.clear_resource_cache_keep_allocation(),
2,
"no matter what - after all we need at least two resources for a diff"
);

platform.clear_resource_cache();
assert_eq!(
platform.clear_resource_cache_keep_allocation(),
0,
"after a proper clearing, the free-list is also emptied, and it won't be recreated"
);

Ok(())
}

Expand Down
12 changes: 6 additions & 6 deletions gix-ref/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ pub struct Namespace(BString);
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum Kind {
/// A ref that points to an object id
Peeled,
/// A ref that points to an object id directly.
Object,
/// A ref that points to another reference, adding a level of indirection.
///
/// It can be resolved to an id using the [`peel_in_place_to_id()`][`crate::file::ReferenceExt::peel_to_id_in_place()`] method.
Expand Down Expand Up @@ -203,8 +203,8 @@ pub enum Category<'a> {
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum Target {
/// A ref that points to an object id
Peeled(ObjectId),
/// A ref that points directly to an object id.
Object(ObjectId),
/// A ref that points to another reference by its validated name, adding a level of indirection.
///
/// Note that this is an extension of gitoxide which will be helpful in logging all reference changes.
Expand All @@ -214,8 +214,8 @@ pub enum Target {
/// Denotes a ref target, equivalent to [`Kind`], but with immutable data.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
pub enum TargetRef<'a> {
/// A ref that points to an object id
Peeled(&'a oid),
/// A ref that points directly to an object id.
Object(&'a oid),
/// A ref that points to another reference by its validated name, adding a level of indirection.
Symbolic(&'a FullNameRef),
}
26 changes: 19 additions & 7 deletions gix-ref/src/peel.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
///
#[allow(clippy::empty_docs)]
pub mod to_id {
use std::path::PathBuf;

use gix_object::bstr::BString;

/// The error returned by [`crate::file::ReferenceExt::peel_to_id_in_place()`].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
FollowToObject(#[from] super::to_object::Error),
#[error("An error occurred when trying to resolve an object a reference points to")]
Find(#[from] gix_object::find::Error),
#[error("Object {oid} as referred to by {name:?} could not be found")]
NotFound { oid: gix_hash::ObjectId, name: BString },
}
}

///
#[allow(clippy::empty_docs)]
pub mod to_object {
use std::path::PathBuf;

use crate::file;

/// The error returned by [`crate::file::ReferenceExt::peel_to_id_in_place()`].
/// The error returned by [`file::ReferenceExt::follow_to_object_in_place_packed()`].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
Expand All @@ -17,9 +33,5 @@ pub mod to_id {
Cycle { start_absolute: PathBuf },
#[error("Refusing to follow more than {max_depth} levels of indirection")]
DepthLimitExceeded { max_depth: usize },
#[error("An error occurred when trying to resolve an object a reference points to")]
Find(#[from] gix_object::find::Error),
#[error("Object {oid} as referred to by {name:?} could not be found")]
NotFound { oid: gix_hash::ObjectId, name: BString },
}
}
5 changes: 3 additions & 2 deletions gix-ref/src/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ pub struct Reference {
pub name: FullName,
/// The target of the reference, either a symbolic reference by full name or a possibly intermediate object by its id.
pub target: Target,
/// The fully peeled object to which this reference ultimately points to. Only guaranteed to be set after
/// The fully peeled object to which this reference ultimately points to after following all symbolic refs and all annotated
/// tags. Only guaranteed to be set after
/// [`Reference::peel_to_id_in_place()`](crate::file::ReferenceExt) was called or if this reference originated
/// from a packed ref.
pub peeled: Option<ObjectId>,
Expand Down Expand Up @@ -48,7 +49,7 @@ mod convert {
fn from(value: packed::Reference<'p>) -> Self {
Reference {
name: value.name.into(),
target: Target::Peeled(value.target()),
target: Target::Object(value.target()),
peeled: value
.object
.map(|hex| ObjectId::from_hex(hex).expect("parser validation")),
Expand Down
2 changes: 1 addition & 1 deletion gix-ref/src/store/file/loose/reference/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl TryFrom<MaybeUnsafeState> for Target {

fn try_from(v: MaybeUnsafeState) -> Result<Self, Self::Error> {
Ok(match v {
MaybeUnsafeState::Id(id) => Target::Peeled(id),
MaybeUnsafeState::Id(id) => Target::Object(id),
MaybeUnsafeState::UnvalidatedPath(name) => {
Target::Symbolic(match gix_validate::reference::name(name.as_ref()) {
Ok(_) => FullName(name),
Expand Down
Loading

0 comments on commit 242fedc

Please sign in to comment.