Skip to content

Commit 07e2a36

Browse files
evanlinjinclaude
andcommitted
perf: skip unselected scan in BnbIter via per-branch cursor
The flamegraph after the delta-aware refactor showed ~32% of run_bnb time spent walking candidate_order and checking Bitset::contains(selected) || Bitset::contains(banned) per element, inlined into insert_new_branches's `cs.unselected().next()`. As BnB descends and more candidates get selected/banned, each .next() call scans further before finding the next viable candidate. But BnB never re-considers a position: each branch's exploration only moves forward in candidate_order. Inclusion advances by 1; exclusion advances past every consecutive same-(value, weight) candidate. So we can store a per-Branch cursor and avoid the scan entirely. Add `Branch::cursor: usize` (the position the branch will expand on next). The init branch starts at 0; insert_new_branches advances past any pre-selected/pre-banned positions on demand, then expands at the located cursor and hands children their new cursors directly. One subtlety: the exclusion branch's same-(value, weight) dedup run now walks raw candidate_order positions, where the old `unselected()` scan skipped already-decided candidates implicitly. The run must do the same explicitly -- skip pre-selected/pre-banned positions (advancing the cursor past them) rather than banning them or letting them end the run. Otherwise a caller-pre-selected candidate that duplicates an excluded one ends up simultaneously selected and banned in the selector that run_bnb hands back, and a pre-decided candidate inside a duplicate run fragments the equivalence class into redundant branches. Covered by `bnb_exclusion_dedup_skips_decided_candidates`. Bench (run_bnb_lowest_fee, n = pool size): n=20 166 us -> 147 us (11%) n=50 5.3 ms -> 4.5 ms (16%) n=100 12.6 ms -> 11.5 ms (9%) n=500 200 ms -> 171 ms (14%) n=1000 365 ms -> 248 ms (32%) (n >= 200 rows are the exhaust-cap group: fixed 100k rounds of frontier expansion, so they measure pure per-round cost.) Largest win at large n where the unselected scan was burning the most time. Flamegraph confirms the unselected-scan hot spot is gone; new top is LowestFee::bound itself (the metric's float math + lookahead). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7524a0d commit 07e2a36

3 files changed

Lines changed: 115 additions & 16 deletions

File tree

Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,8 @@ criterion = "0.5"
2929
[[bench]]
3030
name = "coin_selector"
3131
harness = false
32+
33+
# Enable debug symbols so profilers (perf, samply, flamegraph) can resolve
34+
# function names. No runtime cost.
35+
[profile.bench]
36+
debug = true

src/bnb.rs

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
//! Branch-and-bound search.
2+
//!
3+
//! BnB explores a binary tree where each [`Branch`] expands into two
4+
//! children: an *inclusion* child that selects the candidate at the
5+
//! branch's `cursor`, and an *exclusion* child that bans it along with
6+
//! any same-(value, weight) duplicates that immediately follow.
7+
//!
8+
//! Cursors only advance as we descend — inclusion's child has cursor
9+
//! `parent + 1`; exclusion's jumps past the banned duplicates. That
10+
//! invariant lets `insert_new_branches` skip directly to the next
11+
//! undecided candidate without re-scanning `selected` / `banned`. The
12+
//! only scanning happens to step past pre-selected / pre-banned positions
13+
//! (rare, lazy).
14+
115
use core::cmp::Reverse;
216

317
use crate::{float::Ordf32, Drain, SelectionCache, SelectionView, Target};
@@ -61,6 +75,7 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> {
6175
selector,
6276
cache,
6377
is_exclusion,
78+
cursor,
6479
..
6580
} = branch;
6681

@@ -79,7 +94,7 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> {
7994
};
8095
}
8196

82-
self.insert_new_branches(&selector, &cache);
97+
self.insert_new_branches(&selector, &cache, cursor);
8398
Some(return_val.map(|score| (selector, score)))
8499
}
85100
}
@@ -98,7 +113,7 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> {
98113
}
99114

100115
let cache = SelectionCache::from_selector(&selector);
101-
iter.consider_adding_to_queue(&selector, &cache, false);
116+
iter.consider_adding_to_queue(&selector, &cache, false, 0);
102117

103118
iter
104119
}
@@ -108,6 +123,7 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> {
108123
cs: &CoinSelector<'a>,
109124
cache: &SelectionCache,
110125
is_exclusion: bool,
126+
cursor: usize,
111127
) {
112128
let bound = self
113129
.metric
@@ -123,34 +139,63 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> {
123139
selector: cs.clone(),
124140
cache: cache.clone(),
125141
is_exclusion,
142+
cursor,
126143
});
127144
}
128145
}
129146
}
130147

131-
fn insert_new_branches(&mut self, cs: &CoinSelector<'a>, cache: &SelectionCache) {
132-
let (next_index, next) = match cs.unselected().next() {
133-
Some(c) => c,
134-
None => return, // exhausted
148+
fn insert_new_branches(&mut self, cs: &CoinSelector<'a>, cache: &SelectionCache, start: usize) {
149+
// Find the position to expand on: at or after `start`, whichever is
150+
// the first candidate that's neither selected nor banned. Usually
151+
// this *is* `start` — the only reason to advance is to skip past
152+
// pre-selected/pre-banned candidates (see module-level docs).
153+
let mut iter = cs.candidates().skip(start);
154+
let mut cursor = start;
155+
let (here_idx, here_cand) = loop {
156+
match iter.next() {
157+
None => return, // no more candidates — this branch is a leaf
158+
Some((idx, cand)) => {
159+
if !cs.is_selected(idx) && !cs.banned().contains(idx) {
160+
break (idx, cand);
161+
}
162+
cursor += 1;
163+
}
164+
}
135165
};
166+
// Past here, `iter` is positioned at `cursor + 1`.
136167

137-
// Inclusion branch: selecting `next_index` requires updating the cache.
168+
// Inclusion: descendants explore "this candidate is selected".
138169
let mut inclusion_cs = cs.clone();
139170
let mut inclusion_cache = cache.clone();
140-
inclusion_cs.select(next_index);
141-
inclusion_cache.add(next);
142-
self.consider_adding_to_queue(&inclusion_cs, &inclusion_cache, false);
171+
inclusion_cs.select(here_idx);
172+
inclusion_cache.add(here_cand);
173+
self.consider_adding_to_queue(&inclusion_cs, &inclusion_cache, false, cursor + 1);
143174

144-
// Exclusion branch: only bans, no selection change → cache unchanged.
175+
// Exclusion: descendants explore "this candidate is *not* selected".
176+
// Bans this and every consecutive same-(value, weight) candidate —
177+
// they're equivalent choices, so we deduplicate by handling the
178+
// entire equivalence class in one branch. The cursor jumps past
179+
// all of them.
145180
let mut exclusion_cs = cs.clone();
146-
let to_ban = (next.value, next.weight);
147-
for (ban_index, ban_cand) in cs.unselected() {
148-
if (ban_cand.value, ban_cand.weight) != to_ban {
181+
exclusion_cs.ban(here_idx);
182+
let equiv = (here_cand.value, here_cand.weight);
183+
let mut exclusion_cursor = cursor + 1;
184+
for (idx, cand) in iter {
185+
// Already-decided candidates (pre-selected or banned) must not be banned and must not
186+
// end the equivalence run: the pre-cursor version scanned `unselected()`, which skips
187+
// them entirely. The cursor may still advance past them — they're decided.
188+
if cs.is_selected(idx) || cs.banned().contains(idx) {
189+
exclusion_cursor += 1;
190+
continue;
191+
}
192+
if (cand.value, cand.weight) != equiv {
149193
break;
150194
}
151-
exclusion_cs.ban(ban_index);
195+
exclusion_cs.ban(idx);
196+
exclusion_cursor += 1;
152197
}
153-
self.consider_adding_to_queue(&exclusion_cs, cache, true);
198+
self.consider_adding_to_queue(&exclusion_cs, cache, true, exclusion_cursor);
154199
}
155200
}
156201

@@ -160,6 +205,10 @@ struct Branch<'a> {
160205
selector: CoinSelector<'a>,
161206
cache: SelectionCache,
162207
is_exclusion: bool,
208+
/// Position in `candidate_order` of the candidate whose include /
209+
/// exclude decision creates this branch's two children. See the
210+
/// module-level "Cursor invariant" section.
211+
cursor: usize,
163212
}
164213

165214
impl Ord for Branch<'_> {

tests/bnb.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,51 @@ fn bnb_finds_solution_if_possible_in_n_iter() {
148148
assert_eq!(excess, 0);
149149
}
150150

151+
#[test]
152+
/// The exclusion branch's same-(value, weight) dedup run must skip already-decided candidates
153+
/// instead of banning them (a pre-selected candidate must never end up banned) or letting them
154+
/// end the run early.
155+
///
156+
/// Regression test: candidate 1 is pre-selected by the caller and shares (value, weight) with
157+
/// candidate 0. The optimal solution requires excluding candidate 0, and the dedup run following
158+
/// that exclusion used to also ban the pre-selected candidate 1, contaminating the selector that
159+
/// `run_bnb` hands back.
160+
fn bnb_exclusion_dedup_skips_decided_candidates() {
161+
let candidates = vec![
162+
Candidate::new(500, 100, false), // same (value, weight) as the pre-selected candidate
163+
Candidate::new(500, 100, false), // pre-selected
164+
Candidate::new(400, 100, false),
165+
];
166+
167+
let mut cs = CoinSelector::new(&candidates);
168+
cs.select(1);
169+
170+
let target = Target {
171+
outputs: TargetOutputs {
172+
value_sum: 900,
173+
weight_sum: 0,
174+
n_outputs: 1,
175+
},
176+
fee: TargetFee::ZERO,
177+
};
178+
179+
let _ = cs
180+
.run_bnb(target, MinExcessThenWeight, 1_000)
181+
.expect("must find solution");
182+
183+
// Optimal selection is {1, 2}: candidate 1 is mandatory and adding candidate 2 hits the
184+
// target exactly, while any selection containing candidate 0 overshoots.
185+
assert_eq!(cs.selected_indices().iter().collect::<Vec<_>>(), vec![1, 2]);
186+
// The returned selector must not have any candidate simultaneously selected and banned.
187+
for (idx, _) in cs.selected() {
188+
assert!(
189+
!cs.banned().contains(idx),
190+
"candidate {} is both selected and banned",
191+
idx
192+
);
193+
}
194+
}
195+
151196
proptest! {
152197
#[test]
153198
fn bnb_always_finds_solution_if_possible(num_inputs in 1usize..18, target_value in 0u64..10_000) {

0 commit comments

Comments
 (0)