Merge branch 'improvements'

Author: Byron

gitoxide-core/src/hours/core.rs

@@ -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() {

gitoxide-core/src/query/engine/update.rs

@@ -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))

gix-diff/src/blob/mod.rs

@@ -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 {

gix-diff/src/blob/platform.rs

@@ -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};
@@ -325,6 +325,7 @@ impl Platform {
             old: None,
             new: None,
             diff_cache: Default::default(),
+            free_list: Vec::with_capacity(2),
             options,
             filter,
             filter_mode,
@@ -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
@@ -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()
     }
 }
 
@@ -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,

gix-diff/tests/blob/platform.rs

@@ -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(())
 }
 

gix-ref/src/lib.rs

@@ -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.
@@ -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.
@@ -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),
 }

gix-ref/src/peel.rs

@@ -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 {
@@ -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 },
     }
 }

gix-ref/src/raw.rs

@@ -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>,
@@ -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")),

gix-ref/src/store/file/loose/reference/decode.rs

@@ -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),