fix!: Make topo more similar to Ancestors, but also rename Ancestors to Simple

Author: Byron

gitoxide-core/src/hours/mod.rs

@@ -175,7 +175,7 @@ where
                         }
                         commit_idx += 1;
                     }
-                    Err(gix::traverse::commit::ancestors::Error::Find { .. }) => {
+                    Err(gix::traverse::commit::simple::Error::Find { .. }) => {
                         is_shallow = true;
                         break;
                     }

gitoxide-core/src/pack/create.rs

@@ -130,7 +130,7 @@ where
                 .collect::<Result<Vec<_>, _>>()?;
             let handle = repo.objects.into_shared_arc().to_cache_arc();
             let iter = Box::new(
-                traverse::commit::Ancestors::new(tips, handle.clone())
+                traverse::commit::Simple::new(tips, handle.clone())
                     .map(|res| res.map_err(|err| Box::new(err) as Box<_>).map(|c| c.id))
                     .inspect(move |_| progress.inc()),
             );
@@ -361,7 +361,7 @@ pub mod input_iteration {
     #[derive(Debug, thiserror::Error)]
     pub enum Error {
         #[error("input objects couldn't be iterated completely")]
-        Iteration(#[from] traverse::commit::ancestors::Error),
+        Iteration(#[from] traverse::commit::simple::Error),
         #[error("An error occurred while reading hashes from standard input")]
         InputLinesIo(#[from] std::io::Error),
         #[error("Could not decode hex hash provided on standard input")]

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

@@ -429,7 +429,7 @@ pub fn update(
                         break;
                     }
                 }
-                Err(gix::traverse::commit::ancestors::Error::Find { .. }) => {
+                Err(gix::traverse::commit::simple::Error::Find { .. }) => {
                     writeln!(err, "shallow repository - commit history is truncated").ok();
                     break;
                 }

gix-diff/tests/tree/mod.rs

@@ -133,7 +133,7 @@ mod changes {
             let mut buf = Vec::new();
 
             let head = head_of(db);
-            commit::Ancestors::new(Some(head), &db)
+            commit::Simple::new(Some(head), &db)
                 .collect::<Result<Vec<_>, _>>()
                 .expect("valid iteration")
                 .into_iter()

gix-pack/tests/pack/data/output/count_and_entries.rs

@@ -241,7 +241,7 @@ fn traversals() -> crate::Result {
         .copied()
         {
             let head = hex_to_id("dfcb5e39ac6eb30179808bbab721e8a28ce1b52e");
-            let mut commits = commit::Ancestors::new(Some(head), db.clone())
+            let mut commits = commit::Simple::new(Some(head), db.clone())
                 .map(Result::unwrap)
                 .map(|c| c.id)
                 .collect::<Vec<_>>();

gix-traverse/src/commit/mod.rs

@@ -1,67 +1,43 @@
+//! Provide multiple traversal implementations with different performance envelopes.
+//!
+//! Use [`Simple`] for fast walks that maintain minimal state, or [`Topo`] for a more elaborate traversal.
+use gix_hash::ObjectId;
 use gix_object::FindExt;
+use gix_revwalk::graph::IdMap;
+use gix_revwalk::PriorityQueue;
 use smallvec::SmallVec;
 
 /// A fast iterator over the ancestors of one or more starting commits.
-pub struct Ancestors<Find, Predicate> {
+pub struct Simple<Find, Predicate> {
     objects: Find,
     cache: Option<gix_commitgraph::Graph>,
     predicate: Predicate,
-    state: ancestors::State,
+    state: simple::State,
     parents: Parents,
-    sorting: Sorting,
+    sorting: simple::Sorting,
 }
 
-/// Specify how to sort commits during the [ancestor](Ancestors) traversal.
-///
-/// ### Sample History
-///
-/// The following history will be referred to for explaining how the sort order works, with the number denoting the commit timestamp
-/// (*their X-alignment doesn't matter*).
+/// Simple ancestors traversal, without the need to keep track of graph-state.
+pub mod simple;
+
+/// A commit walker that walks in topographical order, like `git rev-list
+/// --topo-order` or `--date-order` depending on the chosen [`topo::Sorting`].
 ///
-/// ```text
-/// ---1----2----4----7 <- second parent of 8
-///     \              \
-///      3----5----6----8---
-/// ```
-#[derive(Default, Debug, Copy, Clone)]
-pub enum Sorting {
-    /// Commits are sorted as they are mentioned in the commit graph.
-    ///
-    /// In the *sample history* the order would be `8, 6, 7, 5, 4, 3, 2, 1`
-    ///
-    /// ### Note
-    ///
-    /// This is not to be confused with `git log/rev-list --topo-order`, which is notably different from
-    /// as it avoids overlapping branches.
-    #[default]
-    BreadthFirst,
-    /// Commits are sorted by their commit time in descending order, that is newest first.
-    ///
-    /// The sorting applies to all currently queued commit ids and thus is full.
-    ///
-    /// In the *sample history* the order would be `8, 7, 6, 5, 4, 3, 2, 1`
-    ///
-    /// # Performance
-    ///
-    /// This mode benefits greatly from having an object_cache in `find()`
-    /// to avoid having to lookup each commit twice.
-    ByCommitTimeNewestFirst,
-    /// This sorting is similar to `ByCommitTimeNewestFirst`, but adds a cutoff to not return commits older than
-    /// a given time, stopping the iteration once no younger commits is queued to be traversed.
-    ///
-    /// As the query is usually repeated with different cutoff dates, this search mode benefits greatly from an object cache.
-    ///
-    /// In the *sample history* and a cut-off date of 4, the returned list of commits would be `8, 7, 6, 4`
-    ByCommitTimeNewestFirstCutoffOlderThan {
-        /// The amount of seconds since unix epoch, the same value obtained by any `gix_date::Time` structure and the way git counts time.
-        seconds: gix_date::SecondsSinceUnixEpoch,
-    },
+/// Instantiate with [`topo::Builder`].
+pub struct Topo<Find, Predicate> {
+    commit_graph: Option<gix_commitgraph::Graph>,
+    find: Find,
+    predicate: Predicate,
+    indegrees: IdMap<i32>,
+    states: IdMap<topo::WalkFlags>,
+    explore_queue: PriorityQueue<topo::iter::GenAndCommitTime, ObjectId>,
+    indegree_queue: PriorityQueue<topo::iter::GenAndCommitTime, ObjectId>,
+    topo_queue: topo::iter::Queue,
+    parents: Parents,
+    min_gen: u32,
+    buf: Vec<u8>,
 }
 
-/// Simple ancestors traversal
-pub mod ancestors;
-
-// Topological traversal
 pub mod topo;
 
 /// Specify how to handle commit parents during traversal.
@@ -86,8 +62,8 @@ pub struct Info {
     pub id: gix_hash::ObjectId,
     /// All parent ids we have encountered. Note that these will be at most one if [`Parents::First`] is enabled.
     pub parent_ids: ParentIds,
-    /// The time at which the commit was created. It's only `Some(_)` if sorting is not [`Sorting::BreadthFirst`], as the walk
-    /// needs to require the commit-date.
+    /// The time at which the commit was created. It will only be `Some(_)` if the chosen traversal was
+    /// taking dates into consideration.
     pub commit_time: Option<gix_date::SecondsSinceUnixEpoch>,
 }
 

gix-traverse/src/commit/ancestors.rs renamed to gix-traverse/src/commit/simple.rs

@@ -4,7 +4,54 @@ use gix_hashtable::HashSet;
 use smallvec::SmallVec;
 use std::collections::VecDeque;
 
-/// The error is part of the item returned by the [Ancestors](super::Ancestors) iterator.
+/// Specify how to sort commits during a [simple](super::Simple) traversal.
+///
+/// ### Sample History
+///
+/// The following history will be referred to for explaining how the sort order works, with the number denoting the commit timestamp
+/// (*their X-alignment doesn't matter*).
+///
+/// ```text
+/// ---1----2----4----7 <- second parent of 8
+///     \              \
+///      3----5----6----8---
+/// ```
+#[derive(Default, Debug, Copy, Clone)]
+pub enum Sorting {
+    /// Commits are sorted as they are mentioned in the commit graph.
+    ///
+    /// In the *sample history* the order would be `8, 6, 7, 5, 4, 3, 2, 1`
+    ///
+    /// ### Note
+    ///
+    /// This is not to be confused with `git log/rev-list --topo-order`, which is notably different from
+    /// as it avoids overlapping branches.
+    #[default]
+    BreadthFirst,
+    /// Commits are sorted by their commit time in descending order, that is newest first.
+    ///
+    /// The sorting applies to all currently queued commit ids and thus is full.
+    ///
+    /// In the *sample history* the order would be `8, 7, 6, 5, 4, 3, 2, 1`
+    ///
+    /// # Performance
+    ///
+    /// This mode benefits greatly from having an object_cache in `find()`
+    /// to avoid having to lookup each commit twice.
+    ByCommitTimeNewestFirst,
+    /// This sorting is similar to `ByCommitTimeNewestFirst`, but adds a cutoff to not return commits older than
+    /// a given time, stopping the iteration once no younger commits is queued to be traversed.
+    ///
+    /// As the query is usually repeated with different cutoff dates, this search mode benefits greatly from an object cache.
+    ///
+    /// In the *sample history* and a cut-off date of 4, the returned list of commits would be `8, 7, 6, 4`
+    ByCommitTimeNewestFirstCutoffOlderThan {
+        /// The amount of seconds since unix epoch, the same value obtained by any `gix_date::Time` structure and the way git counts time.
+        seconds: gix_date::SecondsSinceUnixEpoch,
+    },
+}
+
+/// The error is part of the item returned by the [Ancestors](super::Simple) iterator.
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
 pub enum Error {
@@ -33,7 +80,7 @@ mod init {
     use gix_object::{CommitRefIter, FindExt};
 
     use super::{
-        super::{Ancestors, Either, Info, ParentIds, Parents, Sorting},
+        super::{simple::Sorting, Either, Info, ParentIds, Parents, Simple},
         collect_parents, Error, State,
     };
 
@@ -60,7 +107,7 @@ mod init {
     }
 
     /// Builder
-    impl<Find, Predicate> Ancestors<Find, Predicate>
+    impl<Find, Predicate> Simple<Find, Predicate>
     where
         Find: gix_object::Find,
     {
@@ -121,7 +168,7 @@ mod init {
     }
 
     /// Lifecyle
-    impl<Find> Ancestors<Find, fn(&oid) -> bool>
+    impl<Find> Simple<Find, fn(&oid) -> bool>
     where
         Find: gix_object::Find,
     {
@@ -139,7 +186,7 @@ mod init {
     }
 
     /// Lifecyle
-    impl<Find, Predicate> Ancestors<Find, Predicate>
+    impl<Find, Predicate> Simple<Find, Predicate>
     where
         Find: gix_object::Find,
         Predicate: FnMut(&oid) -> bool,
@@ -183,7 +230,7 @@ mod init {
     }
 
     /// Access
-    impl<Find, Predicate> Ancestors<Find, Predicate> {
+    impl<Find, Predicate> Simple<Find, Predicate> {
         /// Return an iterator for accessing data of the current commit, parsed lazily.
         pub fn commit_iter(&self) -> CommitRefIter<'_> {
             CommitRefIter::from_bytes(&self.state.buf)
@@ -195,7 +242,7 @@ mod init {
         }
     }
 
-    impl<Find, Predicate> Iterator for Ancestors<Find, Predicate>
+    impl<Find, Predicate> Iterator for Simple<Find, Predicate>
     where
         Find: gix_object::Find,
         Predicate: FnMut(&oid) -> bool,
@@ -228,7 +275,7 @@ mod init {
     }
 
     /// Utilities
-    impl<Find, Predicate> Ancestors<Find, Predicate>
+    impl<Find, Predicate> Simple<Find, Predicate>
     where
         Find: gix_object::Find,
         Predicate: FnMut(&oid) -> bool,
@@ -298,7 +345,7 @@ mod init {
     }
 
     /// Utilities
-    impl<Find, Predicate> Ancestors<Find, Predicate>
+    impl<Find, Predicate> Simple<Find, Predicate>
     where
         Find: gix_object::Find,
         Predicate: FnMut(&oid) -> bool,

gix-traverse/src/commit/topo/init.rs

@@ -1,11 +1,11 @@
 use crate::commit::topo::iter::gen_and_commit_time;
-use crate::commit::topo::{Error, Sorting, Walk, WalkFlags};
-use crate::commit::{find, Info, Parents};
+use crate::commit::topo::{Error, Sorting, WalkFlags};
+use crate::commit::{find, Info, Parents, Topo};
 use gix_hash::{oid, ObjectId};
 use gix_revwalk::graph::IdMap;
 use gix_revwalk::PriorityQueue;
 
-/// Builder for [`Walk`].
+/// Builder for [`Topo`].
 pub struct Builder<Find, Predicate> {
     commit_graph: Option<gix_commitgraph::Graph>,
     find: Find,
@@ -20,7 +20,7 @@ impl<Find> Builder<Find, fn(&oid) -> bool>
 where
     Find: gix_object::Find,
 {
-    /// Create a new `Builder` for a [`Walk`] that reads commits from a repository with `find`.
+    /// Create a new `Builder` for a [`Topo`] that reads commits from a repository with `find`.
     /// starting at the `tips` and ending at the `ends`. Like `git rev-list
     /// --topo-order ^ends... tips...`.
     pub fn from_iters(
@@ -87,11 +87,11 @@ where
         self
     }
 
-    /// Build a new [`Walk`] instance.
+    /// Build a new [`Topo`] instance.
     ///
     /// Note that merely building an instance is currently expensive.
-    pub fn build(self) -> Result<Walk<Find, Predicate>, Error> {
-        let mut w = Walk {
+    pub fn build(self) -> Result<Topo<Find, Predicate>, Error> {
+        let mut w = Topo {
             commit_graph: self.commit_graph,
             find: self.find,
             predicate: self.predicate,

gix-traverse/src/commit/topo/iter.rs

@@ -1,17 +1,17 @@
-use crate::commit::topo::{Error, Sorting, Walk, WalkFlags};
-use crate::commit::{find, Either, Info, Parents};
+use crate::commit::topo::{Error, Sorting, WalkFlags};
+use crate::commit::{find, Either, Info, Parents, Topo};
 use gix_hash::{oid, ObjectId};
 use gix_revwalk::PriorityQueue;
 use smallvec::SmallVec;
 
-pub(super) type GenAndCommitTime = (u32, i64);
+pub(in crate::commit) type GenAndCommitTime = (u32, i64);
 
 // Git's priority queue works as a LIFO stack if no compare function is set,
 // which is the case for `--topo-order.` However, even in that case the initial
 // items of the queue are sorted according to the commit time before beginning
 // the walk.
 #[derive(Debug)]
-pub(super) enum Queue {
+pub(in crate::commit) enum Queue {
     Date(PriorityQueue<i64, Info>),
     Topo(Vec<(i64, Info)>),
 }
@@ -45,7 +45,7 @@ impl Queue {
     }
 }
 
-impl<Find, Predicate> Walk<Find, Predicate>
+impl<Find, Predicate> Topo<Find, Predicate>
 where
     Find: gix_object::Find,
 {
@@ -214,7 +214,7 @@ where
     }
 }
 
-impl<Find, Predicate> Iterator for Walk<Find, Predicate>
+impl<Find, Predicate> Iterator for Topo<Find, Predicate>
 where
     Find: gix_object::Find,
     Predicate: FnMut(&oid) -> bool,