Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3/15] refactor(dag): extract query_stack_commits to Dag #1222

Merged
merged 2 commits into from
Feb 11, 2024
Merged

[3/15] refactor(dag): extract query_stack_commits to Dag #1222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9551e3c
refactor(bisect): pass `Bounds` to `Strategy::midpoint`
arxanas Jan 17, 2024
2e288ac
refactor(dag): extract `query_stack_commits` to `Dag`
arxanas Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions git-branchless-lib/src/core/dag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,30 @@ impl Dag {
})
}

/// Determine the connected components among draft commits (commit "stacks")
/// that intersect with the provided set.
#[instrument]
pub fn query_stack_commits(&self, commit_set: CommitSet) -> eyre::Result<CommitSet> {
let draft_commits = self.query_draft_commits()?;
let stack_roots = self.query_roots(draft_commits.clone())?;
let stack_ancestors = self.query_range(stack_roots, commit_set)?;
let stack = self
// Note that for a graph like
//
// ```
// O
// |
// o A
// | \
// | o B
// |
// @ C
// ```
// this will return `{A, B, C}`, not just `{A, C}`.
.query_range(stack_ancestors, draft_commits.clone())?;
Ok(stack)
}

/// Wrapper around DAG method.
#[instrument]
pub fn query_all(&self) -> eyre::Result<CommitSet> {
Expand Down
25 changes: 3 additions & 22 deletions git-branchless-revset/src/builtins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,28 +290,9 @@ fn fn_draft(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult {
#[instrument]
fn fn_stack(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult {
let arg = eval0_or_1(ctx, name, args)?.unwrap_or_else(|| ctx.dag.head_commit.clone());
let draft_commits = ctx
.dag
.query_draft_commits()
.map_err(EvalError::OtherError)?;
let stack_roots = ctx.dag.query_roots(draft_commits.clone())?;
let stack_ancestors = ctx.dag.query_range(stack_roots, arg)?;
let stack = ctx
.dag
// Note that for a graph like
//
// ```
// O
// |
// o A
// | \
// | o B
// |
// @ C
// ```
// this will return `{A, B, C}`, not just `{A, C}`.
.query_range(stack_ancestors, draft_commits.clone())?;
Ok(stack)
ctx.dag
.query_stack_commits(arg)
.map_err(EvalError::OtherError)
}

type MatcherFn = dyn Fn(&Repo, &Commit) -> Result<bool, PatternError> + Sync + Send;
Expand Down
8 changes: 5 additions & 3 deletions scm-bisect/examples/guessing_game.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::cmp::Ordering;
use std::collections::HashSet;
use std::convert::Infallible;
use std::io;
use std::ops::RangeInclusive;
Expand Down Expand Up @@ -38,10 +37,13 @@ impl search::Strategy<Graph> for Strategy {
fn midpoint(
&self,
_graph: &Graph,
success_bounds: &HashSet<Node>,
failure_bounds: &HashSet<Node>,
bounds: &search::Bounds<Node>,
_statuses: &IndexMap<Node, search::Status>,
) -> Result<Option<Node>, Self::Error> {
let search::Bounds {
success: success_bounds,
failure: failure_bounds,
} = bounds;
let lower_bound = success_bounds
.iter()
.max()
Expand Down
7 changes: 5 additions & 2 deletions scm-bisect/src/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,13 @@ impl<G: BasicSourceControlGraph> search::Strategy<G> for BasicStrategy {
fn midpoint(
&self,
graph: &G,
success_bounds: &HashSet<G::Node>,
failure_bounds: &HashSet<G::Node>,
bounds: &search::Bounds<G::Node>,
statuses: &IndexMap<G::Node, search::Status>,
) -> Result<Option<G::Node>, G::Error> {
let search::Bounds {
success: success_bounds,
failure: failure_bounds,
} = bounds;
let mut nodes_to_search = {
let implied_success_nodes = graph.ancestors_all(success_bounds.clone())?;
let implied_failure_nodes = graph.descendants_all(failure_bounds.clone())?;
Expand Down
68 changes: 32 additions & 36 deletions scm-bisect/src/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ pub trait Strategy<G: Graph>: Debug {
fn midpoint(
&self,
graph: &G,
success_bounds: &HashSet<G::Node>,
failure_bounds: &HashSet<G::Node>,
bounds: &Bounds<G::Node>,
statuses: &IndexMap<G::Node, Status>,
) -> Result<Option<G::Node>, Self::Error>;
}
Expand Down Expand Up @@ -274,8 +273,7 @@ impl<G: Graph> Search<G> {

#[derive(Debug)]
struct State<G: Graph> {
success_bounds: HashSet<G::Node>,
failure_bounds: HashSet<G::Node>,
bounds: Bounds<G::Node>,
statuses: IndexMap<G::Node, Status>,
}

Expand All @@ -292,24 +290,16 @@ impl<G: Graph> Search<G> {
fn next(&mut self) -> Option<Self::Item> {
while let Some(state) = self.states.pop_front() {
debug!(?state, "Popped speculation state");
let State {
success_bounds,
failure_bounds,
statuses,
} = state;

let node = match self.strategy.midpoint(
self.graph,
&success_bounds,
&failure_bounds,
&statuses,
) {
let State { bounds, statuses } = state;

let node = match self.strategy.midpoint(self.graph, &bounds, &statuses) {
Ok(Some(node)) => node,
Ok(None) => continue,
Err(err) => return Some(Err(SearchError::Strategy(err))),
};

for success_node in success_bounds.iter() {
let Bounds { success, failure } = bounds;
for success_node in success.iter() {
match self.graph.is_ancestor(node.clone(), success_node.clone()) {
Ok(true) => {
return Some(Err(SearchError::AlreadySearchedMidpoint {
Expand All @@ -321,7 +311,7 @@ impl<G: Graph> Search<G> {
Err(err) => return Some(Err(SearchError::Graph(err))),
}
}
for failure_node in failure_bounds.iter() {
for failure_node in failure.iter() {
match self.graph.is_ancestor(failure_node.clone(), node.clone()) {
Ok(true) => {
return Some(Err(SearchError::AlreadySearchedMidpoint {
Expand All @@ -336,14 +326,16 @@ impl<G: Graph> Search<G> {

// Speculate failure:
self.states.push_back(State {
success_bounds: success_bounds.clone(),
failure_bounds: {
let mut failure_bounds = failure_bounds.clone();
failure_bounds.insert(node.clone());
match self.graph.simplify_failure_bounds(failure_bounds) {
Ok(bounds) => bounds,
Err(err) => return Some(Err(SearchError::Graph(err))),
}
bounds: Bounds {
success: success.clone(),
failure: {
let mut failure_bounds = failure.clone();
failure_bounds.insert(node.clone());
match self.graph.simplify_failure_bounds(failure_bounds) {
Ok(bounds) => bounds,
Err(err) => return Some(Err(SearchError::Graph(err))),
}
},
},
statuses: {
let mut statuses = statuses.clone();
Expand All @@ -354,15 +346,17 @@ impl<G: Graph> Search<G> {

// Speculate success:
self.states.push_back(State {
success_bounds: {
let mut success_bounds = success_bounds.clone();
success_bounds.insert(node.clone());
match self.graph.simplify_success_bounds(success_bounds) {
Ok(bounds) => bounds,
Err(err) => return Some(Err(SearchError::Graph(err))),
}
bounds: Bounds {
success: {
let mut success_bounds = success.clone();
success_bounds.insert(node.clone());
match self.graph.simplify_success_bounds(success_bounds) {
Ok(bounds) => bounds,
Err(err) => return Some(Err(SearchError::Graph(err))),
}
},
failure: failure.clone(),
},
failure_bounds: failure_bounds.clone(),
statuses: {
let mut statuses = statuses.clone();
statuses.insert(node.clone(), Status::Success);
Expand All @@ -379,8 +373,10 @@ impl<G: Graph> Search<G> {
}

let initial_state = State {
success_bounds: success_bounds.clone(),
failure_bounds: failure_bounds.clone(),
bounds: Bounds {
success: success_bounds.clone(),
failure: failure_bounds.clone(),
},
statuses: self.nodes.clone(),
};
let iter = Iter {
Expand Down
Loading