Merge pull request #1564 from Byron/improvements

improvements

Author: Byron

gitoxide-core/src/repository/merge_base.rs

@@ -20,7 +20,8 @@ pub fn merge_base(
         .collect::<Result<_, _>>()?;
 
     let cache = repo.commit_graph_if_enabled()?;
-    let bases = repo.merge_bases_many_with_cache(first_id, &other_ids, cache.as_ref())?;
+    let mut graph = repo.revision_graph(cache.as_ref());
+    let bases = repo.merge_bases_many_with_graph(first_id, &other_ids, &mut graph)?;
     if bases.is_empty() {
         bail!("No base found for {first} and {others}", others = others.join(", "))
     }

gix-negotiate/src/consecutive.rs

@@ -23,7 +23,7 @@ impl Algorithm {
         let mut is_common = false;
         let mut has_mark = false;
         if let Some(commit) = graph
-            .try_lookup_or_insert_commit(id, |data| {
+            .get_or_insert_commit(id, |data| {
                 has_mark = data.flags.intersects(mark);
                 data.flags |= mark;
                 is_common = data.flags.contains(Flags::COMMON);
@@ -47,7 +47,7 @@ impl Algorithm {
     ) -> Result<(), Error> {
         let mut is_common = false;
         if let Some(commit) = graph
-            .try_lookup_or_insert_commit(id, |data| is_common = data.flags.contains(Flags::COMMON))?
+            .get_or_insert_commit(id, |data| is_common = data.flags.contains(Flags::COMMON))?
             .filter(|_| !is_common)
         {
             let mut queue = gix_revwalk::PriorityQueue::from_iter(Some((commit.commit_time, (id, 0_usize))));
@@ -64,11 +64,11 @@ impl Algorithm {
                 {
                     self.add_to_queue(id, Flags::SEEN, graph)?;
                 } else if matches!(ancestors, Ancestors::AllUnseen) || generation < 2 {
-                    if let Some(commit) = graph.try_lookup_or_insert_commit(id, |_| {})? {
+                    if let Some(commit) = graph.get_or_insert_commit(id, |_| {})? {
                         for parent_id in commit.parents.clone() {
                             let mut prev_flags = Flags::default();
                             if let Some(parent) = graph
-                                .try_lookup_or_insert_commit(parent_id, |data| {
+                                .get_or_insert_commit(parent_id, |data| {
                                     prev_flags = data.flags;
                                     data.flags |= Flags::COMMON;
                                 })?

gix-negotiate/src/lib.rs

@@ -144,4 +144,4 @@ pub trait Negotiator {
 }
 
 /// An error that happened during any of the methods on a [`Negotiator`].
-pub type Error = gix_revwalk::graph::try_lookup_or_insert_default::Error;
+pub type Error = gix_revwalk::graph::get_or_insert_default::Error;

gix-negotiate/src/skipping.rs

@@ -20,7 +20,7 @@ impl Default for Algorithm {
 impl Algorithm {
     /// Add `id` to our priority queue and *add* `flags` to it.
     fn add_to_queue(&mut self, id: ObjectId, mark: Flags, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
-        let commit = graph.try_lookup_or_insert_commit(id, |entry| {
+        let commit = graph.get_or_insert_commit(id, |entry| {
             entry.flags |= mark | Flags::SEEN;
         })?;
         if let Some(timestamp) = commit.map(|c| c.commit_time) {
@@ -35,15 +35,15 @@ impl Algorithm {
     fn mark_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         let mut is_common = false;
         if let Some(commit) = graph
-            .try_lookup_or_insert_commit(id, |entry| {
+            .get_or_insert_commit(id, |entry| {
                 is_common = entry.flags.contains(Flags::COMMON);
                 entry.flags |= Flags::COMMON;
             })?
             .filter(|_| !is_common)
         {
             let mut queue = gix_revwalk::PriorityQueue::from_iter(Some((commit.commit_time, id)));
             while let Some(id) = queue.pop_value() {
-                if let Some(commit) = graph.try_lookup_or_insert_commit(id, |entry| {
+                if let Some(commit) = graph.get_or_insert_commit(id, |entry| {
                     if !entry.flags.contains(Flags::POPPED) {
                         self.non_common_revs -= 1;
                     }
@@ -56,7 +56,7 @@ impl Algorithm {
                         }
                         let mut was_unseen_or_common = false;
                         if let Some(parent) = graph
-                            .try_lookup_or_insert_commit(parent_id, |entry| {
+                            .get_or_insert_commit(parent_id, |entry| {
                                 was_unseen_or_common =
                                     !entry.flags.contains(Flags::SEEN) || entry.flags.contains(Flags::COMMON);
                                 entry.flags |= Flags::COMMON;

gix-revision/src/merge_base.rs

@@ -18,19 +18,15 @@ bitflags::bitflags! {
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
 pub enum Error {
-    #[error(transparent)]
-    IterParents(#[from] gix_revwalk::graph::commit::iter_parents::Error),
-    #[error("A commit could not be found")]
-    FindExistingCommit(#[from] gix_object::find::existing_iter::Error),
-    #[error("A commit could not be decoded during traversal")]
-    Decode(#[from] gix_object::decode::Error),
+    #[error("A commit could not be inserted into the graph")]
+    InsertCommit(#[from] gix_revwalk::graph::get_or_insert_default::Error),
 }
 
 pub(crate) mod function {
     use super::Error;
     use crate::{merge_base::Flags, Graph, PriorityQueue};
     use gix_hash::ObjectId;
-    use gix_revwalk::graph::LazyCommit;
+    use gix_revwalk::graph;
     use std::cmp::Ordering;
 
     /// Given a commit at `first` id, traverse the commit `graph` and return all possible merge-base between it and `others`,
@@ -39,19 +35,23 @@ pub(crate) mod function {
     ///
     /// Note that this function doesn't do any work if `first` is contained in `others`, which is when `first` will be returned
     /// as only merge-base right away. This is even the case if some commits of `others` are disjoint.
+    ///
+    /// # Performance
+    ///
+    /// For repeated calls, be sure to re-use `graph` as its content will be kept and reused for a great speed-up. The contained flags
+    /// will automatically be cleared.
     pub fn merge_base(
         first: ObjectId,
         others: &[ObjectId],
-        graph: &mut Graph<'_, '_, Flags>,
+        graph: &mut Graph<'_, '_, graph::Commit<Flags>>,
     ) -> Result<Option<Vec<ObjectId>>, Error> {
         let _span = gix_trace::coarse!("gix_revision::merge_base()", ?first, ?others);
         if others.is_empty() || others.contains(&first) {
             return Ok(Some(vec![first]));
         }
 
-        graph.clear();
+        graph.clear_commit_data(|f| *f = Flags::empty());
         let bases = paint_down_to_common(first, others, graph)?;
-        graph.clear();
 
         let bases = remove_redundant(&bases, graph)?;
         Ok((!bases.is_empty()).then_some(bases))
@@ -61,11 +61,12 @@ pub(crate) mod function {
     /// That way, we return only the topologically most recent commits in `commits`.
     fn remove_redundant(
         commits: &[(ObjectId, GenThenTime)],
-        graph: &mut Graph<'_, '_, Flags>,
+        graph: &mut Graph<'_, '_, graph::Commit<Flags>>,
     ) -> Result<Vec<ObjectId>, Error> {
         if commits.is_empty() {
             return Ok(Vec::new());
         }
+        graph.clear_commit_data(|f| *f = Flags::empty());
         let _span = gix_trace::detail!("gix_revision::remove_redundant()", num_commits = %commits.len());
         let sorted_commits = {
             let mut v = commits.to_vec();
@@ -77,36 +78,46 @@ pub(crate) mod function {
 
         let mut walk_start = Vec::with_capacity(commits.len());
         for (id, _) in commits {
-            graph.insert_parents_with_lookup(id, &mut |parent_id, parent_data, maybe_flags| -> Result<_, Error> {
-                if maybe_flags.map_or(true, |flags| !flags.contains(Flags::STALE)) {
-                    walk_start.push((parent_id, GenThenTime::try_from(parent_data)?));
-                }
-                Ok(Flags::empty())
-            })?;
-            graph.insert(*id, Flags::RESULT);
+            let commit = graph.get_mut(id).expect("previously added");
+            commit.data |= Flags::RESULT;
+            for parent_id in commit.parents.clone() {
+                graph.get_or_insert_full_commit(parent_id, |parent| {
+                    // prevent double-addition
+                    if !parent.data.contains(Flags::STALE) {
+                        parent.data |= Flags::STALE;
+                        walk_start.push((parent_id, GenThenTime::from(&*parent)));
+                    }
+                })?;
+            }
         }
         walk_start.sort_by(|a, b| a.0.cmp(&b.0));
+        // allow walking everything at first.
+        walk_start
+            .iter_mut()
+            .for_each(|(id, _)| graph.get_mut(id).expect("added previously").data.remove(Flags::STALE));
         let mut count_still_independent = commits.len();
 
         let mut stack = Vec::new();
         while let Some((commit_id, commit_info)) = walk_start.pop().filter(|_| count_still_independent > 1) {
             stack.clear();
-            graph.insert(commit_id, Flags::STALE);
+            graph.get_mut(&commit_id).expect("added").data |= Flags::STALE;
             stack.push((commit_id, commit_info));
 
             while let Some((commit_id, commit_info)) = stack.last().copied() {
-                let flags = graph.get_mut(&commit_id).expect("all commits have been added");
-                if flags.contains(Flags::RESULT) {
-                    flags.remove(Flags::RESULT);
+                let commit = graph.get_mut(&commit_id).expect("all commits have been added");
+                let commit_parents = commit.parents.clone();
+                if commit.data.contains(Flags::RESULT) {
+                    commit.data.remove(Flags::RESULT);
                     count_still_independent -= 1;
                     if count_still_independent <= 1 {
                         break;
                     }
-                    if commit_id == sorted_commits[min_gen_pos].0 {
+                    if *commit_id == *sorted_commits[min_gen_pos].0 {
                         while min_gen_pos < commits.len() - 1
                             && graph
                                 .get(&sorted_commits[min_gen_pos].0)
                                 .expect("already added")
+                                .data
                                 .contains(Flags::STALE)
                         {
                             min_gen_pos += 1;
@@ -120,87 +131,81 @@ pub(crate) mod function {
                     continue;
                 }
 
-                let mut pushed_one_parent = false;
-                graph.insert_parents_with_lookup(&commit_id, &mut |parent_id,
-                                                                    parent_data,
-                                                                    maybe_flags|
-                 -> Result<_, Error> {
-                    let is_new_parent = !pushed_one_parent
-                        && maybe_flags.map_or(true, |flags| {
-                            let res = !flags.contains(Flags::STALE);
-                            *flags |= Flags::STALE;
-                            res
-                        });
-                    if is_new_parent {
-                        stack.push((parent_id, GenThenTime::try_from(parent_data)?));
-                        pushed_one_parent = true;
+                let previous_len = stack.len();
+                for parent_id in &commit_parents {
+                    if graph
+                        .get_or_insert_full_commit(*parent_id, |parent| {
+                            if !parent.data.contains(Flags::STALE) {
+                                parent.data |= Flags::STALE;
+                                stack.push((*parent_id, GenThenTime::from(&*parent)));
+                            }
+                        })?
+                        .is_some()
+                    {
+                        break;
                     }
-                    Ok(Flags::STALE)
-                })?;
+                }
 
-                if !pushed_one_parent {
+                if previous_len == stack.len() {
                     stack.pop();
                 }
             }
         }
 
         Ok(commits
             .iter()
-            .filter_map(|(id, _info)| graph.get(id).filter(|flags| !flags.contains(Flags::STALE)).map(|_| *id))
+            .filter_map(|(id, _info)| {
+                graph
+                    .get(id)
+                    .filter(|commit| !commit.data.contains(Flags::STALE))
+                    .map(|_| *id)
+            })
             .collect())
     }
 
     fn paint_down_to_common(
         first: ObjectId,
         others: &[ObjectId],
-        graph: &mut Graph<'_, '_, Flags>,
+        graph: &mut Graph<'_, '_, graph::Commit<Flags>>,
     ) -> Result<Vec<(ObjectId, GenThenTime)>, Error> {
         let mut queue = PriorityQueue::<GenThenTime, ObjectId>::new();
-        graph.insert_data(first, |commit| -> Result<_, Error> {
-            queue.insert(commit.try_into()?, first);
-            Ok(Flags::COMMIT1)
+        graph.get_or_insert_full_commit(first, |commit| {
+            commit.data |= Flags::COMMIT1;
+            queue.insert(GenThenTime::from(&*commit), first);
         })?;
 
         for other in others {
-            graph.insert_data(*other, |commit| -> Result<_, Error> {
-                queue.insert(commit.try_into()?, *other);
-                Ok(Flags::COMMIT2)
+            graph.get_or_insert_full_commit(*other, |commit| {
+                commit.data |= Flags::COMMIT2;
+                queue.insert(GenThenTime::from(&*commit), *other);
             })?;
         }
 
         let mut out = Vec::new();
-        while queue
-            .iter_unordered()
-            .any(|id| graph.get(id).map_or(false, |data| !data.contains(Flags::STALE)))
-        {
+        while queue.iter_unordered().any(|id| {
+            graph
+                .get(id)
+                .map_or(false, |commit| !commit.data.contains(Flags::STALE))
+        }) {
             let (info, commit_id) = queue.pop().expect("we have non-stale");
-            let flags_mut = graph.get_mut(&commit_id).expect("everything queued is in graph");
-            let mut flags_without_result = *flags_mut & (Flags::COMMIT1 | Flags::COMMIT2 | Flags::STALE);
+            let commit = graph.get_mut(&commit_id).expect("everything queued is in graph");
+            let mut flags_without_result = commit.data & (Flags::COMMIT1 | Flags::COMMIT2 | Flags::STALE);
             if flags_without_result == (Flags::COMMIT1 | Flags::COMMIT2) {
-                if !flags_mut.contains(Flags::RESULT) {
-                    *flags_mut |= Flags::RESULT;
+                if !commit.data.contains(Flags::RESULT) {
+                    commit.data |= Flags::RESULT;
                     out.push((commit_id, info));
                 }
                 flags_without_result |= Flags::STALE;
             }
 
-            graph.insert_parents_with_lookup(&commit_id, &mut |parent_id, parent, ex_flags| -> Result<_, Error> {
-                let queue_info = match ex_flags {
-                    Some(ex_flags) => {
-                        if (*ex_flags & flags_without_result) != flags_without_result {
-                            *ex_flags |= flags_without_result;
-                            Some(GenThenTime::try_from(parent)?)
-                        } else {
-                            None
-                        }
+            for parent_id in commit.parents.clone() {
+                graph.get_or_insert_full_commit(parent_id, |parent| {
+                    if (parent.data & flags_without_result) != flags_without_result {
+                        parent.data |= flags_without_result;
+                        queue.insert(GenThenTime::from(&*parent), parent_id);
                     }
-                    None => Some(GenThenTime::try_from(parent)?),
-                };
-                if let Some(info) = queue_info {
-                    queue.insert(info, parent_id);
-                }
-                Ok(flags_without_result)
-            })?;
+                })?;
+            }
         }
 
         Ok(out)
@@ -215,15 +220,12 @@ pub(crate) mod function {
         time: gix_date::SecondsSinceUnixEpoch,
     }
 
-    impl TryFrom<gix_revwalk::graph::LazyCommit<'_, '_>> for GenThenTime {
-        type Error = gix_object::decode::Error;
-
-        fn try_from(commit: LazyCommit<'_, '_>) -> Result<Self, Self::Error> {
-            let (generation, timestamp) = commit.generation_and_timestamp()?;
-            Ok(GenThenTime {
-                generation: generation.unwrap_or(gix_commitgraph::GENERATION_NUMBER_INFINITY),
-                time: timestamp,
-            })
+    impl From<&graph::Commit<Flags>> for GenThenTime {
+        fn from(commit: &graph::Commit<Flags>) -> Self {
+            GenThenTime {
+                generation: commit.generation.unwrap_or(gix_commitgraph::GENERATION_NUMBER_INFINITY),
+                time: commit.commit_time,
+            }
         }
     }
 

gix-revision/tests/merge_base/mod.rs

@@ -17,7 +17,6 @@ mod baseline {
                     .then(|| gix_commitgraph::Graph::from_info_dir(&odb.store_ref().path().join("info")).unwrap());
                 for expected in parse_expectations(&baseline_path)? {
                     let mut graph = gix_revision::Graph::new(&odb, cache.as_ref());
-
                     let actual = merge_base(expected.first, &expected.others, &mut graph)?;
                     assert_eq!(
                         actual,
@@ -27,6 +26,17 @@ mod baseline {
                         input = expected.plain_input
                     );
                 }
+                let mut graph = gix_revision::Graph::new(&odb, cache.as_ref());
+                for expected in parse_expectations(&baseline_path)? {
+                    let actual = merge_base(expected.first, &expected.others, &mut graph)?;
+                    assert_eq!(
+                        actual,
+                        expected.bases,
+                        "sample (reused graph) {file:?}:{input}",
+                        file = baseline_path.with_extension("").file_name(),
+                        input = expected.plain_input
+                    );
+                }
             }
         }
         assert_ne!(count, 0, "there must be at least one baseline");

gix-revwalk/src/graph/mod.rs

@@ -30,7 +30,7 @@ mod errors {
     }
 
     ///
-    pub mod try_lookup_or_insert_default {
+    pub mod get_or_insert_default {
         use crate::graph::commit::to_owned;
 
         /// The error returned by [`try_lookup_or_insert_default()`](crate::Graph::try_lookup_or_insert_default()).
@@ -44,7 +44,7 @@ mod errors {
         }
     }
 }
-pub use errors::{insert_parents, try_lookup_or_insert_default};
+pub use errors::{get_or_insert_default, insert_parents};
 use gix_date::SecondsSinceUnixEpoch;
 
 /// The generation away from the HEAD of graph, useful to limit algorithms by topological depth as well.
@@ -68,7 +68,7 @@ impl<'find, 'cache, T: Default> Graph<'find, 'cache, T> {
         &mut self,
         id: gix_hash::ObjectId,
         update_data: impl FnOnce(&mut T),
-    ) -> Result<Option<LazyCommit<'_, 'cache>>, try_lookup_or_insert_default::Error> {
+    ) -> Result<Option<LazyCommit<'_, 'cache>>, get_or_insert_default::Error> {
         self.try_lookup_or_insert_default(id, T::default, update_data)
     }
 }
@@ -225,19 +225,19 @@ impl<'find, 'cache, T> Graph<'find, 'cache, T> {
     }
 }
 
-/// commit access
+/// Commit based methods
 impl<'find, 'cache, T> Graph<'find, 'cache, Commit<T>> {
-    /// Lookup `id` without failing if the commit doesn't exist, and assure that `id` is inserted into our set
-    /// with a commit with `new_data()` assigned.
+    /// Lookup `id` in the graph, but insert it if it's not yet present by looking it up without failing if the commit doesn't exist.
+    /// Call `new_data()` to obtain data for a newly inserted commit.
     /// `update_data(data)` gets run either on existing or on new data.
     ///
     /// Note that none of the data updates happen if `id` didn't exist.
-    pub fn try_lookup_or_insert_commit_default(
+    pub fn get_or_insert_commit_default(
         &mut self,
         id: gix_hash::ObjectId,
         new_data: impl FnOnce() -> T,
         update_data: impl FnOnce(&mut T),
-    ) -> Result<Option<&mut Commit<T>>, try_lookup_or_insert_default::Error> {
+    ) -> Result<Option<&mut Commit<T>>, get_or_insert_default::Error> {
         match self.map.entry(id) {
             gix_hashtable::hash_map::Entry::Vacant(entry) => {
                 let res = try_lookup(&id, &*self.find, self.cache, &mut self.buf)?;
@@ -255,24 +255,56 @@ impl<'find, 'cache, T> Graph<'find, 'cache, Commit<T>> {
         };
         Ok(self.map.get_mut(&id))
     }
+
+    /// For each stored commit, call `clear` on its data.
+    pub fn clear_commit_data(&mut self, mut clear: impl FnMut(&mut T)) {
+        self.map.values_mut().for_each(|c| clear(&mut c.data));
+    }
 }
 
-/// commit access
+/// Commit based methods
 impl<'find, 'cache, T: Default> Graph<'find, 'cache, Commit<T>> {
-    /// Lookup `id` without failing if the commit doesn't exist or `id` isn't a commit,
-    /// and assure that `id` is inserted into our set with a commit and default data assigned.
+    /// Lookup `id` in the graph, but insert it if it's not yet present by looking it up without failing if the commit doesn't exist.
+    /// Newly inserted commits are populated with default data.
     /// `update_data(data)` gets run either on existing or on new data.
     ///
     /// Note that none of the data updates happen if `id` didn't exist.
     ///
     /// If only commit data is desired without the need for attaching custom data, use
     /// [`try_lookup(id).to_owned()`][Graph::try_lookup()] instead.
-    pub fn try_lookup_or_insert_commit(
+    pub fn get_or_insert_commit(
         &mut self,
         id: gix_hash::ObjectId,
         update_data: impl FnOnce(&mut T),
-    ) -> Result<Option<&mut Commit<T>>, try_lookup_or_insert_default::Error> {
-        self.try_lookup_or_insert_commit_default(id, T::default, update_data)
+    ) -> Result<Option<&mut Commit<T>>, get_or_insert_default::Error> {
+        self.get_or_insert_commit_default(id, T::default, update_data)
+    }
+
+    /// Lookup `id` in the graph, but insert it if it's not yet present by looking it up without failing if the commit doesn't exist.
+    /// `update_commit(commit)` gets run either on existing or on new data.
+    ///
+    /// Note that none of the data updates happen if `id` didn't exist in the graph.
+    pub fn get_or_insert_full_commit(
+        &mut self,
+        id: gix_hash::ObjectId,
+        update_commit: impl FnOnce(&mut Commit<T>),
+    ) -> Result<Option<&mut Commit<T>>, get_or_insert_default::Error> {
+        match self.map.entry(id) {
+            gix_hashtable::hash_map::Entry::Vacant(entry) => {
+                let res = try_lookup(&id, &*self.find, self.cache, &mut self.buf)?;
+                let commit = match res {
+                    None => return Ok(None),
+                    Some(commit) => commit,
+                };
+                let mut commit = commit.to_owned(T::default)?;
+                update_commit(&mut commit);
+                entry.insert(commit);
+            }
+            gix_hashtable::hash_map::Entry::Occupied(mut entry) => {
+                update_commit(entry.get_mut());
+            }
+        };
+        Ok(self.map.get_mut(&id))
     }
 }
 
@@ -293,7 +325,7 @@ impl<'find, 'cache, T> Graph<'find, 'cache, T> {
         id: gix_hash::ObjectId,
         default: impl FnOnce() -> T,
         update_data: impl FnOnce(&mut T),
-    ) -> Result<Option<LazyCommit<'_, 'cache>>, try_lookup_or_insert_default::Error> {
+    ) -> Result<Option<LazyCommit<'_, 'cache>>, get_or_insert_default::Error> {
         let res = try_lookup(&id, &*self.find, self.cache, &mut self.buf)?;
         Ok(res.map(|commit| {
             match self.map.entry(id) {

gix/src/remote/connection/fetch/negotiate.rs

@@ -16,7 +16,7 @@ pub enum Error {
     #[error("We were unable to figure out what objects the server should send after {rounds} round(s)")]
     NegotiationFailed { rounds: usize },
     #[error(transparent)]
-    LookupCommitInGraph(#[from] gix_revwalk::graph::try_lookup_or_insert_default::Error),
+    LookupCommitInGraph(#[from] gix_revwalk::graph::get_or_insert_default::Error),
     #[error(transparent)]
     InitRefsIterator(#[from] crate::reference::iter::init::Error),
     #[error(transparent)]
@@ -114,7 +114,7 @@ pub(crate) fn mark_complete_and_common_ref(
         }
 
         if let Some(commit) = want_id
-            .and_then(|id| graph.try_lookup_or_insert_commit(id.into(), |_| {}).transpose())
+            .and_then(|id| graph.get_or_insert_commit(id.into(), |_| {}).transpose())
             .transpose()?
         {
             remote_ref_target_known[mapping_idx] = true;
@@ -271,7 +271,7 @@ fn mark_recent_complete_commits(
         for parent_id in commit.parents.clone() {
             let mut was_complete = false;
             if let Some(parent) = graph
-                .try_lookup_or_insert_commit(parent_id, |md| {
+                .get_or_insert_commit(parent_id, |md| {
                     was_complete = md.flags.contains(Flags::COMPLETE);
                     md.flags |= Flags::COMPLETE;
                 })?
@@ -296,7 +296,7 @@ fn mark_all_refs_in_repo(
         let id = local_ref.id().detach();
         let mut is_complete = false;
         if let Some(commit) = graph
-            .try_lookup_or_insert_commit(id, |md| {
+            .get_or_insert_commit(id, |md| {
                 is_complete = md.flags.contains(Flags::COMPLETE);
                 md.flags |= mark;
             })?

gix/src/repository/mod.rs

@@ -93,8 +93,8 @@ pub mod merge_base {
 
 ///
 #[cfg(feature = "revision")]
-pub mod merge_base_with_cache {
-    /// The error returned by [Repository::merge_base_with_cache()](crate::Repository::merge_base_with_cache()).
+pub mod merge_base_with_graph {
+    /// The error returned by [Repository::merge_base_with_cache()](crate::Repository::merge_base_with_graph()).
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {

gix/src/repository/revision.rs

@@ -39,7 +39,7 @@ impl crate::Repository {
     /// Obtain the best merge-base between commit `one` and `two`, or fail if there is none.
     ///
     /// # Performance
-    /// For repeated calls, prefer [`merge_base_with_cache()`](crate::Repository::merge_base_with_cache()).
+    /// For repeated calls, prefer [`merge_base_with_cache()`](crate::Repository::merge_base_with_graph()).
     /// Also be sure to [set an object cache](crate::Repository::object_cache_size_if_unset) to accelerate repeated commit lookups.
     #[cfg(feature = "revision")]
     pub fn merge_base(
@@ -60,47 +60,44 @@ impl crate::Repository {
     }
 
     /// Obtain the best merge-base between commit `one` and `two`, or fail if there is none, providing a
-    /// commit-graph `cache` to potentially greatly accelerate the operation.
+    /// commit-graph `graph` to potentially greatly accelerate the operation by reusing graphs from previous runs.
     ///
     /// # Performance
     /// Be sure to [set an object cache](crate::Repository::object_cache_size_if_unset) to accelerate repeated commit lookups.
     #[cfg(feature = "revision")]
-    pub fn merge_base_with_cache(
+    pub fn merge_base_with_graph(
         &self,
         one: impl Into<gix_hash::ObjectId>,
         two: impl Into<gix_hash::ObjectId>,
-        cache: Option<&gix_commitgraph::Graph>,
-    ) -> Result<Id<'_>, super::merge_base_with_cache::Error> {
+        graph: &mut gix_revwalk::Graph<'_, '_, gix_revwalk::graph::Commit<gix_revision::merge_base::Flags>>,
+    ) -> Result<Id<'_>, super::merge_base_with_graph::Error> {
         use crate::prelude::ObjectIdExt;
         let one = one.into();
         let two = two.into();
-        let mut graph = self.revision_graph(cache);
-        let bases = gix_revision::merge_base(one, &[two], &mut graph)?.ok_or(
-            super::merge_base_with_cache::Error::NotFound {
+        let bases =
+            gix_revision::merge_base(one, &[two], graph)?.ok_or(super::merge_base_with_graph::Error::NotFound {
                 first: one,
                 second: two,
-            },
-        )?;
+            })?;
         Ok(bases[0].attach(self))
     }
 
     /// Obtain all merge-bases between commit `one` and `others`, or an empty list if there is none, providing a
-    /// commit-graph `cache` to potentially greatly accelerate the operation.
+    /// commit-graph `graph` to potentially greatly accelerate the operation.
     ///
     /// # Performance
     /// Be sure to [set an object cache](crate::Repository::object_cache_size_if_unset) to accelerate repeated commit lookups.
     #[doc(alias = "merge_bases_many", alias = "git2")]
     #[cfg(feature = "revision")]
-    pub fn merge_bases_many_with_cache(
+    pub fn merge_bases_many_with_graph(
         &self,
         one: impl Into<gix_hash::ObjectId>,
         others: &[gix_hash::ObjectId],
-        cache: Option<&gix_commitgraph::Graph>,
+        graph: &mut gix_revwalk::Graph<'_, '_, gix_revwalk::graph::Commit<gix_revision::merge_base::Flags>>,
     ) -> Result<Vec<Id<'_>>, gix_revision::merge_base::Error> {
         use crate::prelude::ObjectIdExt;
         let one = one.into();
-        let mut graph = self.revision_graph(cache);
-        Ok(gix_revision::merge_base(one, others, &mut graph)?
+        Ok(gix_revision::merge_base(one, others, graph)?
             .unwrap_or_default()
             .into_iter()
             .map(|id| id.attach(self))