Skip to content

Commit 242fedc

Browse files
ByronByron
committed
Merge branch 'improvements'
2 parents 7b7902e + f944e49 commit 242fedc

File tree

47 files changed

+935
-216
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+935
-216
lines changed

‎gitoxide-core/src/hours/core.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub fn spawn_tree_delta_threads<'scope>(
105105
for chunk in rx {
106106
for (commit_idx, parent_commit, commit) in chunk {
107107
if let Some(cache) = cache.as_mut() {
108-
cache.clear_resource_cache();
108+
cache.clear_resource_cache_keep_allocation();
109109
}
110110
commits.fetch_add(1, Ordering::Relaxed);
111111
if gix::interrupt::is_triggered() {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ pub fn update(
204204
Some(c) => c,
205205
None => continue,
206206
};
207-
rewrite_cache.clear_resource_cache();
208-
diff_cache.clear_resource_cache();
207+
rewrite_cache.clear_resource_cache_keep_allocation();
208+
diff_cache.clear_resource_cache_keep_allocation();
209209
from.changes()?
210210
.track_path()
211211
.track_rewrites(Some(rewrites))

‎gix-diff/src/blob/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ pub struct Platform {
119119
/// That way, expensive rewrite-checks with NxM matrix checks would be as fast as possible,
120120
/// avoiding duplicate work.
121121
diff_cache: HashMap<platform::CacheKey, platform::CacheValue>,
122+
/// A list of previously used buffers, ready for re-use.
123+
free_list: Vec<Vec<u8>>,
122124
}
123125

124126
mod impls {

‎gix-diff/src/blob/platform.rs‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use std::{io::Write, process::Stdio};
2-
31
use bstr::{BStr, BString, ByteSlice};
2+
use std::cmp::Ordering;
3+
use std::{io::Write, process::Stdio};
44

55
use super::Algorithm;
66
use crate::blob::{pipeline, Pipeline, Platform, ResourceKind};
@@ -325,6 +325,7 @@ impl Platform {
325325
old: None,
326326
new: None,
327327
diff_cache: Default::default(),
328+
free_list: Vec::with_capacity(2),
328329
options,
329330
filter,
330331
filter_mode,
@@ -542,7 +543,7 @@ impl Platform {
542543

543544
/// Every call to [set_resource()](Self::set_resource()) will keep the diffable data in memory, and that will never be cleared.
544545
///
545-
/// Use this method to clear the cache, releasing memory. Note that this will also loose all information about resources
546+
/// Use this method to clear the cache, releasing memory. Note that this will also lose all information about resources
546547
/// which means diffs would fail unless the resources are set again.
547548
///
548549
/// 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 {
551552
self.old = None;
552553
self.new = None;
553554
self.diff_cache.clear();
555+
self.free_list.clear();
556+
}
557+
558+
/// Every call to [set_resource()](Self::set_resource()) will keep the diffable data in memory, and that will never be cleared.
559+
///
560+
/// Use this method to clear the cache, but keep the previously used buffers around for later re-use.
561+
///
562+
/// If there are more buffers on the free-list than there are stored sources, we half that amount each time this method is called,
563+
/// or keep as many resources as were previously stored, or 2 buffers, whatever is larger.
564+
/// 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
565+
/// number of previously stored resources.
566+
///
567+
/// Returns the number of available buffers.
568+
pub fn clear_resource_cache_keep_allocation(&mut self) -> usize {
569+
self.old = None;
570+
self.new = None;
571+
572+
let diff_cache = std::mem::take(&mut self.diff_cache);
573+
match self.free_list.len().cmp(&diff_cache.len()) {
574+
Ordering::Less => {
575+
let to_take = diff_cache.len() - self.free_list.len();
576+
self.free_list
577+
.extend(diff_cache.into_values().map(|v| v.buffer).take(to_take));
578+
}
579+
Ordering::Equal => {}
580+
Ordering::Greater => {
581+
let new_len = (self.free_list.len() / 2).max(diff_cache.len()).max(2);
582+
self.free_list.truncate(new_len);
583+
}
584+
}
585+
self.free_list.len()
554586
}
555587
}
556588

@@ -591,7 +623,7 @@ impl Platform {
591623
kind,
592624
rela_path: rela_path.to_owned(),
593625
})?;
594-
let mut buf = Vec::new();
626+
let mut buf = self.free_list.pop().unwrap_or_default();
595627
let out = self.filter.convert_to_diffable(
596628
&id,
597629
mode,

‎gix-diff/tests/blob/platform.rs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,35 @@ fn resources_of_worktree_and_odb_and_check_link() -> crate::Result {
121121
"Also obvious that symlinks are definitely special, but it's what git does as well"
122122
);
123123

124-
platform.clear_resource_cache();
124+
assert_eq!(
125+
platform.clear_resource_cache_keep_allocation(),
126+
3,
127+
"some buffers are retained and reused"
128+
);
125129
assert_eq!(
126130
platform.resources(),
127131
None,
128132
"clearing the cache voids resources and one has to set it up again"
129133
);
130134

135+
assert_eq!(
136+
platform.clear_resource_cache_keep_allocation(),
137+
2,
138+
"doing this again keeps 2 buffers"
139+
);
140+
assert_eq!(
141+
platform.clear_resource_cache_keep_allocation(),
142+
2,
143+
"no matter what - after all we need at least two resources for a diff"
144+
);
145+
146+
platform.clear_resource_cache();
147+
assert_eq!(
148+
platform.clear_resource_cache_keep_allocation(),
149+
0,
150+
"after a proper clearing, the free-list is also emptied, and it won't be recreated"
151+
);
152+
131153
Ok(())
132154
}
133155

‎gix-ref/src/lib.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ pub struct Namespace(BString);
150150
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
151151
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
152152
pub enum Kind {
153-
/// A ref that points to an object id
154-
Peeled,
153+
/// A ref that points to an object id directly.
154+
Object,
155155
/// A ref that points to another reference, adding a level of indirection.
156156
///
157157
/// 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> {
203203
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
204204
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
205205
pub enum Target {
206-
/// A ref that points to an object id
207-
Peeled(ObjectId),
206+
/// A ref that points directly to an object id.
207+
Object(ObjectId),
208208
/// A ref that points to another reference by its validated name, adding a level of indirection.
209209
///
210210
/// Note that this is an extension of gitoxide which will be helpful in logging all reference changes.
@@ -214,8 +214,8 @@ pub enum Target {
214214
/// Denotes a ref target, equivalent to [`Kind`], but with immutable data.
215215
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
216216
pub enum TargetRef<'a> {
217-
/// A ref that points to an object id
218-
Peeled(&'a oid),
217+
/// A ref that points directly to an object id.
218+
Object(&'a oid),
219219
/// A ref that points to another reference by its validated name, adding a level of indirection.
220220
Symbolic(&'a FullNameRef),
221221
}

‎gix-ref/src/peel.rs‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,29 @@
11
///
22
#[allow(clippy::empty_docs)]
33
pub mod to_id {
4-
use std::path::PathBuf;
5-
64
use gix_object::bstr::BString;
75

6+
/// The error returned by [`crate::file::ReferenceExt::peel_to_id_in_place()`].
7+
#[derive(Debug, thiserror::Error)]
8+
#[allow(missing_docs)]
9+
pub enum Error {
10+
#[error(transparent)]
11+
FollowToObject(#[from] super::to_object::Error),
12+
#[error("An error occurred when trying to resolve an object a reference points to")]
13+
Find(#[from] gix_object::find::Error),
14+
#[error("Object {oid} as referred to by {name:?} could not be found")]
15+
NotFound { oid: gix_hash::ObjectId, name: BString },
16+
}
17+
}
18+
19+
///
20+
#[allow(clippy::empty_docs)]
21+
pub mod to_object {
22+
use std::path::PathBuf;
23+
824
use crate::file;
925

10-
/// The error returned by [`crate::file::ReferenceExt::peel_to_id_in_place()`].
26+
/// The error returned by [`file::ReferenceExt::follow_to_object_in_place_packed()`].
1127
#[derive(Debug, thiserror::Error)]
1228
#[allow(missing_docs)]
1329
pub enum Error {
@@ -17,9 +33,5 @@ pub mod to_id {
1733
Cycle { start_absolute: PathBuf },
1834
#[error("Refusing to follow more than {max_depth} levels of indirection")]
1935
DepthLimitExceeded { max_depth: usize },
20-
#[error("An error occurred when trying to resolve an object a reference points to")]
21-
Find(#[from] gix_object::find::Error),
22-
#[error("Object {oid} as referred to by {name:?} could not be found")]
23-
NotFound { oid: gix_hash::ObjectId, name: BString },
2436
}
2537
}

‎gix-ref/src/raw.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ pub struct Reference {
1010
pub name: FullName,
1111
/// The target of the reference, either a symbolic reference by full name or a possibly intermediate object by its id.
1212
pub target: Target,
13-
/// The fully peeled object to which this reference ultimately points to. Only guaranteed to be set after
13+
/// The fully peeled object to which this reference ultimately points to after following all symbolic refs and all annotated
14+
/// tags. Only guaranteed to be set after
1415
/// [`Reference::peel_to_id_in_place()`](crate::file::ReferenceExt) was called or if this reference originated
1516
/// from a packed ref.
1617
pub peeled: Option<ObjectId>,
@@ -48,7 +49,7 @@ mod convert {
4849
fn from(value: packed::Reference<'p>) -> Self {
4950
Reference {
5051
name: value.name.into(),
51-
target: Target::Peeled(value.target()),
52+
target: Target::Object(value.target()),
5253
peeled: value
5354
.object
5455
.map(|hex| ObjectId::from_hex(hex).expect("parser validation")),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ impl TryFrom<MaybeUnsafeState> for Target {
3535

3636
fn try_from(v: MaybeUnsafeState) -> Result<Self, Self::Error> {
3737
Ok(match v {
38-
MaybeUnsafeState::Id(id) => Target::Peeled(id),
38+
MaybeUnsafeState::Id(id) => Target::Object(id),
3939
MaybeUnsafeState::UnvalidatedPath(name) => {
4040
Target::Symbolic(match gix_validate::reference::name(name.as_ref()) {
4141
Ok(_) => FullName(name),

0 commit comments

Comments
 (0)