Skip to content

Commit c545d71

Browse files
committed
fix!: Tree::lookup_entry() looses its buf argument.
The buffer will now be previded from the free-list of the repository.
1 parent a10e7c2 commit c545d71

File tree

3 files changed

+13
-21
lines changed

3 files changed

+13
-21
lines changed

Diff for: gix/src/object/tree/mod.rs

+11-14
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,25 @@ impl<'repo> Tree<'repo> {
4141

4242
/// Follow a sequence of `path` components starting from this instance, and look them up one by one until the last component
4343
/// is looked up and its tree entry is returned.
44-
/// Use `buf` as temporary location for sub-trees to avoid allocating a temporary buffer for each lookup.
4544
///
4645
/// # Performance Notes
4746
///
4847
/// Searching tree entries is currently done in sequence, which allows to the search to be allocation free. It would be possible
4948
/// to reuse a vector and use a binary search instead, which might be able to improve performance over all.
5049
/// However, a benchmark should be created first to have some data and see which trade-off to choose here.
5150
///
52-
pub fn lookup_entry<I, P>(&self, path: I, buf: &mut Vec<u8>) -> Result<Option<Entry<'repo>>, find::existing::Error>
51+
pub fn lookup_entry<I, P>(&self, path: I) -> Result<Option<Entry<'repo>>, find::existing::Error>
5352
where
5453
I: IntoIterator<Item = P>,
5554
P: PartialEq<BStr>,
5655
{
57-
let mut path = path.into_iter().peekable();
56+
let mut buf = self.repo.shared_empty_buf();
5857
buf.clear();
58+
59+
let mut path = path.into_iter().peekable();
5960
buf.extend_from_slice(&self.data);
6061
while let Some(component) = path.next() {
61-
match TreeRefIter::from_bytes(buf)
62+
match TreeRefIter::from_bytes(&buf)
6263
.filter_map(Result::ok)
6364
.find(|entry| component.eq(entry.filename))
6465
{
@@ -70,7 +71,7 @@ impl<'repo> Tree<'repo> {
7071
}));
7172
} else {
7273
let next_id = entry.oid.to_owned();
73-
let obj = self.repo.objects.find(&next_id, buf)?;
74+
let obj = self.repo.objects.find(&next_id, &mut buf)?;
7475
if !obj.kind.is_tree() {
7576
return Ok(None);
7677
}
@@ -134,17 +135,13 @@ impl<'repo> Tree<'repo> {
134135
pub fn lookup_entry_by_path(
135136
&self,
136137
relative_path: impl AsRef<std::path::Path>,
137-
buf: &mut Vec<u8>,
138138
) -> Result<Option<Entry<'repo>>, find::existing::Error> {
139139
use crate::bstr::ByteSlice;
140-
self.lookup_entry(
141-
relative_path.as_ref().components().map(|c: std::path::Component<'_>| {
142-
gix_path::os_str_into_bstr(c.as_os_str())
143-
.unwrap_or_else(|_| "".into())
144-
.as_bytes()
145-
}),
146-
buf,
147-
)
140+
self.lookup_entry(relative_path.as_ref().components().map(|c: std::path::Component<'_>| {
141+
gix_path::os_str_into_bstr(c.as_os_str())
142+
.unwrap_or_else(|_| "".into())
143+
.as_bytes()
144+
}))
148145
}
149146

150147
/// Like [`Self::peel_to_entry()`], but takes a `Path` directly via `relative_path`, a path relative to this tree.

Diff for: gix/src/repository/object.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl crate::Repository {
139139
}
140140
}
141141

142-
fn shared_empty_buf(&self) -> std::cell::RefMut<'_, Vec<u8>> {
142+
pub(crate) fn shared_empty_buf(&self) -> std::cell::RefMut<'_, Vec<u8>> {
143143
let mut bufs = self.bufs.borrow_mut();
144144
if bufs.last().is_none() {
145145
bufs.push(Vec::with_capacity(512));

Diff for: gix/tests/object/tree/mod.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ fn find_entry() -> crate::Result {
1717
fn lookup_entry_by_path() -> crate::Result {
1818
let repo = named_subrepo_opts("make_worktree_repo.sh", "repo", gix::open::Options::isolated())?;
1919
let tree = repo.head_commit()?.tree()?;
20-
assert_eq!(
21-
tree.lookup_entry_by_path("dir/c", &mut Vec::new())?
22-
.expect("present")
23-
.filename(),
24-
"c"
25-
);
20+
assert_eq!(tree.lookup_entry_by_path("dir/c")?.expect("present").filename(), "c");
2621
Ok(())
2722
}

0 commit comments

Comments
 (0)