Skip to content

Commit

Permalink
Add note on why we consume the tree for looking up an entry. (#470)
Browse files Browse the repository at this point in the history
The disadvantage of doing that is that we copy the name, which
allocates, instead of just returning it by reference.
  • Loading branch information
Byron committed Sep 20, 2022
1 parent 5ec714f commit b285097
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
7 changes: 6 additions & 1 deletion git-repository/src/object/tree/iter.rs
Expand Up @@ -6,7 +6,7 @@ pub struct EntryRef<'repo, 'a> {
/// The actual entry ref we are wrapping.
pub inner: git_object::tree::EntryRef<'a>,

repo: &'repo Repository,
pub(crate) repo: &'repo Repository,
}

impl<'repo, 'a> EntryRef<'repo, 'a> {
Expand All @@ -24,6 +24,11 @@ impl<'repo, 'a> EntryRef<'repo, 'a> {
pub fn id(&self) -> crate::Id<'repo> {
crate::Id::from_id(self.inner.oid, self.repo)
}

/// Return the entries id, without repository connection.
pub fn oid(&self) -> git_hash::ObjectId {
self.inner.oid.to_owned()
}
}

impl<'repo, 'a> std::fmt::Display for EntryRef<'repo, 'a> {
Expand Down
6 changes: 6 additions & 0 deletions git-repository/src/object/tree/mod.rs
Expand Up @@ -31,6 +31,12 @@ impl<'repo> Tree<'repo> {
/// Searching tree entries is currently done in sequence, which allows to the search to be allocation free. It would be possible
/// to re-use a vector and use a binary search instead, which might be able to improve performance over all.
/// However, a benchmark should be created first to have some data and see which trade-off to choose here.
///
/// # Why is this consunming?
///
/// The borrow checker shows pathological behaviour in loops that mutate a buffer, but also want to return from it.
/// Workarounds include keeping an index and doing a separate access to the memory, which seems hard to do here without
/// re-parsing the entries.
pub fn lookup_entry<I, P>(mut self, path: I) -> Result<Option<git_object::tree::Entry>, find::existing::Error>
where
I: IntoIterator<Item = P>,
Expand Down

0 comments on commit b285097

Please sign in to comment.