Merge pull request #9488 from Byron/graph-segmentation

integration-checks: performance

Author: Byron

Cargo.lock

@@ -158,13 +158,11 @@ pub struct WorkspaceStack {
 pub struct WorkspaceStackBranch {
     /// The name of the branch.
     pub ref_name: gix::refs::FullName,
-    /// Archived represents the state when series/branch has been integrated and is below the merge base with the current target branch.
-    /// This would occur when the branch has been merged at the remote and the workspace has been updated with that change.
-    ///
-    /// Note that this is a cache to help speed up certain operations.
-    /// NOTE: This is more like a proof of concept and for backwards compatibility - maybe we will make it go away.
-    // TODO: given that most operations require a graph walk, will this really be necessary if a graph cache is used consistently?
-    //       Staleness can be a problem if targets can be changed after the fact. At least we'd need to recompute it.
+    /// If `true`, the branch is now underneath the lower-base of the workspace after a workspace update.
+    /// This means it's not interesting anymore, by all means, but we'd still have to keep it available and list
+    /// these segments as being part of the workspace when creating PRs. Their descriptions contain references
+    /// to archived segments, which simply shouldn't disappear just yet.
+    /// However, they may disappear once the whole stack has been integrated and the workspace has moved past it.
     pub archived: bool,
 }
 

crates/but-core/src/ref_metadata.rs

@@ -97,28 +97,65 @@ pub struct Target {
 impl Target {
     /// Return `None` if `ref_name` wasn't found as segment in `graph`.
     /// This can happen if a reference is configured, but not actually present as reference.
-    fn from_ref_name(ref_name: &gix::refs::FullName, graph: &Graph) -> Option<Self> {
+    /// Note that `commits_ahead` isn't set yet, see [`Self::compute_and_set_commits_ahead()`].
+    fn from_ref_name_without_commits_ahead(
+        ref_name: &gix::refs::FullName,
+        graph: &Graph,
+    ) -> Option<Self> {
         let target_segment = graph.inner.node_indices().find_map(|n| {
             let s = &graph[n];
             (s.ref_name.as_ref() == Some(ref_name)).then_some(s)
         })?;
         Some(Target {
             ref_name: ref_name.to_owned(),
             segment_index: target_segment.id,
-            commits_ahead: {
-                // Find all remote commits but stop traversing when there is segments without remotes.
-                let mut count = 0;
-                graph.visit_all_segments_until(target_segment.id, Direction::Outgoing, |s| {
-                    let remote_commits = s.commits.iter().filter(|c| c.flags.is_remote()).count();
-                    count += remote_commits;
-                    remote_commits != s.commits.len()
-                });
-                count
-            },
+            commits_ahead: 0,
+        })
+    }
+
+    fn compute_and_set_commits_ahead(
+        &mut self,
+        graph: &Graph,
+        lower_bound_segment: Option<SegmentIndex>,
+    ) {
+        let lower_bound = lower_bound_segment.map(|sidx| (sidx, graph[sidx].generation));
+        self.commits_ahead = 0;
+        Self::visit_upstream_commits(graph, self.segment_index, lower_bound, |s| {
+            self.commits_ahead += s.commits.len();
         })
     }
 }
 
+/// Utilities
+impl Target {
+    /// Visit all segments whose commits would be considered 'upstream', or part of the target branch
+    /// whose tip is identified with `target_segment`. The `lower_bound_segment_and_generation` is another way
+    /// to stop the traversal.
+    pub fn visit_upstream_commits(
+        graph: &Graph,
+        target_segment: SegmentIndex,
+        lower_bound_segment_and_generation: Option<(SegmentIndex, usize)>,
+        mut visit: impl FnMut(&Segment),
+    ) {
+        graph.visit_all_segments_until(target_segment, Direction::Outgoing, |s| {
+            let prune = true;
+            if lower_bound_segment_and_generation.is_some_and(
+                |(lower_bound, lower_bound_generation)| {
+                    s.id == lower_bound || s.generation > lower_bound_generation
+                },
+            ) || s
+                .commits
+                .iter()
+                .any(|c| c.flags.contains(CommitFlags::InWorkspace))
+            {
+                return prune;
+            }
+            visit(s);
+            !prune
+        });
+    }
+}
+
 impl Graph {
     /// Analyse the current graph starting at its [entrypoint](Self::lookup_entrypoint()).
     ///
@@ -196,9 +233,9 @@ impl Graph {
             id: ws_tip_segment.id,
             kind,
             stacks: vec![],
-            target: metadata
-                .as_ref()
-                .and_then(|md| Target::from_ref_name(md.target_ref.as_ref()?, self)),
+            target: metadata.as_ref().and_then(|md| {
+                Target::from_ref_name_without_commits_ahead(md.target_ref.as_ref()?, self)
+            }),
             extra_target: self.extra_target,
             metadata,
             lower_bound_segment_id: None,
@@ -352,6 +389,9 @@ impl Graph {
             );
         }
 
+        if let Some(target) = ws.target.as_mut() {
+            target.compute_and_set_commits_ahead(self, ws.lower_bound_segment_id);
+        }
         ws.mark_remote_reachability()?;
         Ok(ws)
     }
@@ -834,6 +874,8 @@ impl std::fmt::Debug for Workspace<'_> {
             .field("id", &self.id.index())
             .field("stacks", &self.stacks)
             .field("metadata", &self.metadata)
+            .field("target", &self.target)
+            .field("extra_target", &self.extra_target)
             .finish()
     }
 }

crates/but-graph/src/projection/workspace.rs

@@ -30,6 +30,8 @@ itertools = "0.14"
 url = { version = "2.5.4", features = ["serde"] }
 md5 = "0.8.0"
 tracing.workspace = true
+# For SPMC channel
+flume = "0.11.1"
 
 [dev-dependencies]
 but-testsupport.workspace = true