From ba72ee0f069b7df96f7543fe2d97612c98544733 Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Thu, 22 Aug 2024 09:41:34 +0200
Subject: [PATCH] fix!: better peeling performance for reference traversal.

This is done by keeping a packed-buffer around and reusing it, instead
of re-checking it every time.

For this to work, the `peeled()` function on the `reference::Iter` can now fail
as it has to open a packed-refs snapshot.
---
 gix/src/reference/iter.rs                       | 17 ++++++++++++-----
 gix/src/remote/connection/fetch/negotiate.rs    |  2 +-
 .../revision/spec/parse/delegate/navigate.rs    |  1 +
 gix/tests/repository/reference.rs               |  2 +-
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs
index f5bf99306fc..7b10ff48f13 100644
--- a/gix/src/reference/iter.rs
+++ b/gix/src/reference/iter.rs
@@ -14,6 +14,7 @@ pub struct Platform<'r> {
 /// An iterator over references, with or without filter.
 pub struct Iter<'r> {
     inner: gix_ref::file::iter::LooseThenPacked<'r, 'r>,
+    peel_with_packed: Option<gix_ref::file::packed::SharedBufferSnapshot>,
     peel: bool,
     repo: &'r crate::Repository,
 }
@@ -22,6 +23,7 @@ impl<'r> Iter<'r> {
     fn new(repo: &'r crate::Repository, platform: gix_ref::file::iter::LooseThenPacked<'r, 'r>) -> Self {
         Iter {
             inner: platform,
+            peel_with_packed: None,
             peel: false,
             repo,
         }
@@ -80,9 +82,10 @@ impl<'r> Iter<'r> {
     ///
     /// Doing this is necessary as the packed-refs buffer is already held by the iterator, disallowing the consumer of the iterator
     /// to peel the returned references themselves.
-    pub fn peeled(mut self) -> Self {
+    pub fn peeled(mut self) -> Result<Self, gix_ref::packed::buffer::open::Error> {
+        self.peel_with_packed = self.repo.refs.cached_packed_buffer()?;
         self.peel = true;
-        self
+        Ok(self)
     }
 }
 
@@ -95,9 +98,13 @@ impl<'r> Iterator for Iter<'r> {
                 .and_then(|mut r| {
                     if self.peel {
                         let repo = &self.repo;
-                        r.peel_to_id_in_place(&repo.refs, &repo.objects)
-                            .map_err(|err| Box::new(err) as Box<dyn std::error::Error + Send + Sync + 'static>)
-                            .map(|_| r)
+                        r.peel_to_id_in_place_packed(
+                            &repo.refs,
+                            &repo.objects,
+                            self.peel_with_packed.as_ref().map(|p| &***p),
+                        )
+                        .map_err(|err| Box::new(err) as Box<dyn std::error::Error + Send + Sync + 'static>)
+                        .map(|_| r)
                     } else {
                         Ok(r)
                     }
diff --git a/gix/src/remote/connection/fetch/negotiate.rs b/gix/src/remote/connection/fetch/negotiate.rs
index d9e73fe7dbc..ade90bdbcb6 100644
--- a/gix/src/remote/connection/fetch/negotiate.rs
+++ b/gix/src/remote/connection/fetch/negotiate.rs
@@ -291,7 +291,7 @@ fn mark_all_refs_in_repo(
     mark: Flags,
 ) -> Result<(), Error> {
     let _span = gix_trace::detail!("mark_all_refs");
-    for local_ref in repo.references()?.all()?.peeled() {
+    for local_ref in repo.references()?.all()?.peeled()? {
         let local_ref = local_ref?;
         let id = local_ref.id().detach();
         let mut is_complete = false;
diff --git a/gix/src/revision/spec/parse/delegate/navigate.rs b/gix/src/revision/spec/parse/delegate/navigate.rs
index 43f40654f40..4dd24800726 100644
--- a/gix/src/revision/spec/parse/delegate/navigate.rs
+++ b/gix/src/revision/spec/parse/delegate/navigate.rs
@@ -240,6 +240,7 @@ impl<'repo> delegate::Navigate for Delegate<'repo> {
                             .rev_walk(
                                 references
                                     .peeled()
+                                    .ok()?
                                     .filter_map(Result::ok)
                                     .filter(|r| r.id().header().ok().map_or(false, |obj| obj.kind().is_commit()))
                                     .filter_map(|r| r.detach().peeled),
diff --git a/gix/tests/repository/reference.rs b/gix/tests/repository/reference.rs
index 40668673022..94e9e8574d2 100644
--- a/gix/tests/repository/reference.rs
+++ b/gix/tests/repository/reference.rs
@@ -156,7 +156,7 @@ mod iter_references {
         assert_eq!(
             repo.references()?
                 .prefixed("refs/heads/")?
-                .peeled()
+                .peeled()?
                 .filter_map(Result::ok)
                 .map(|r| (
                     r.name().as_bstr().to_string(),