From 721f5e63afda57c0abdb041171c984be74d9c1e9 Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Sun, 8 Nov 2026 22:15:00 +0100 Subject: [PATCH] perf(construction): reuse candidate buffers across placements Descriptor-placement scalar construction allocated about ten short-lived buffers per placement: a fresh candidate store, the candidate value list (plus an index vector and a second collected vector under ascending-key value order), and the slot's candidate values collected into a Vec before they were visited. Construction time then depended on where glibc placed those buffers. A placement cursor now takes finished placements back (EntityPlacerCursor::recycle, called by the construction phase after each step) and reuses their candidate store and value buffer. Ascending-key value order sorts the buffer in place, keeping offered order between equal keys. Static scalar slots visit candidate values without collecting them, and buffer fillers copy them in one bulk call. The __bench placement cursor gains recycle, so the gate's construction_full case drives placements the way the phase does. Against v0.24.0 on the isolated zero-regression gate: construction first fit -48% instructions, construction full -12%, ruin -43%, with every case producing identical results. Closes #222 --- crates/solverforge-solver/WIREFRAME.md | 2 +- .../construction_cases.rs | 2 + .../src/bench/construction.rs | 6 ++ .../src/builder/context/scalar/variable.rs | 90 +++++++++++++------ .../src/builder/context/scalar_access.rs | 26 +++++- .../heuristic/selector/move_selector/arena.rs | 1 - .../phase/construction/phase/phase_type.rs | 1 + .../src/phase/construction/placer.rs | 12 +++ .../construction/runtime_slots/global.rs | 2 +- .../construction/runtime_slots/placement.rs | 70 +++++++++------ .../tests/construction_buffer_reuse.rs | 84 +++++++++++++++++ 11 files changed, 238 insertions(+), 58 deletions(-) create mode 100644 crates/solverforge-solver/tests/construction_buffer_reuse.rs diff --git a/crates/solverforge-solver/WIREFRAME.md b/crates/solverforge-solver/WIREFRAME.md index ff149e5b..368bc284 100644 --- a/crates/solverforge-solver/WIREFRAME.md +++ b/crates/solverforge-solver/WIREFRAME.md @@ -422,7 +422,7 @@ modules are private, so an item has exactly one path: ### Hidden modules -- `__bench` (`#[doc(hidden)]`): handles over the compiled runtime's own cursors that `benches/selector_cursor_gate.rs` drives. No compatibility promise covers them. `CompiledRuntimeRunner::compile(&SolverConfig, SearchContext)` runs the compilation, preparation, and lowering a configured solve runs (the crate-private `compile_runtime_runner` that `try_run_solver_with_config_and_search` also calls) and implements `Phase` by delegating to the compiled runtime phase runner. `into_local_search_neighborhood()` moves out the first phase's lowered acceptor/forager neighborhood as `CompiledNeighborhood`, whose `open_cursor(director, MoveStreamContext)` returns `CompiledNeighborhoodCursor`: the composition cursor a local-search step pulls from, with `next_candidate(&mut director)` lending the solve's provider resources per pull, `visit_candidate(id, CandidateVisitor)`, and `release_candidate(id)`. `into_scalar_construction_placer()` moves out the first phase's descriptor-placement scalar construction as `CompiledScalarPlacer`, whose `open_cursor` yields `CompiledScalarPlacementCursor` and `CompiledScalarPlacement` (`next_candidate`, `visit_candidate`, `candidate_assignment`, `release_candidate`) over the placer the construction phase drives. `CandidateVisitor` inspects one live candidate through the public `Move` contract without naming the runtime's private move carriers. +- `__bench` (`#[doc(hidden)]`): handles over the compiled runtime's own cursors that `benches/selector_cursor_gate.rs` drives. No compatibility promise covers them. `CompiledRuntimeRunner::compile(&SolverConfig, SearchContext)` runs the compilation, preparation, and lowering a configured solve runs (the crate-private `compile_runtime_runner` that `try_run_solver_with_config_and_search` also calls) and implements `Phase` by delegating to the compiled runtime phase runner. `into_local_search_neighborhood()` moves out the first phase's lowered acceptor/forager neighborhood as `CompiledNeighborhood`, whose `open_cursor(director, MoveStreamContext)` returns `CompiledNeighborhoodCursor`: the composition cursor a local-search step pulls from, with `next_candidate(&mut director)` lending the solve's provider resources per pull, `visit_candidate(id, CandidateVisitor)`, and `release_candidate(id)`. `into_scalar_construction_placer()` moves out the first phase's descriptor-placement scalar construction as `CompiledScalarPlacer`, whose `open_cursor` yields `CompiledScalarPlacementCursor` (`next_placement`, and `recycle`, which hands a finished placement back so the next one reuses its candidate store and value buffer, as the construction phase does after each step) and `CompiledScalarPlacement` (`next_candidate`, `visit_candidate`, `candidate_assignment`, `release_candidate`) over the placer the construction phase drives. `CandidateVisitor` inspects one live candidate through the public `Move` contract without naming the runtime's private move carriers. - `__private` (`#[doc(hidden)]`): the host-provider contracts that `solverforge-bridge` re-exports for host-language bindings: `RuntimeCandidateMetric`, `RuntimeCandidateMetricBinding`, `RuntimeCandidateMetricRegistry`, `RuntimeProviderRegistry`, `ProviderResolutionError`, `RawProviderCandidate`, `RawProviderEdit`, `RuntimeConflictRepairProviderBinding`, `RuntimeHostCompoundProvider`, `RuntimeHostProviderErrorBoundary`, `RuntimeProviderLimits`, `RuntimeScalarGroupProviderBinding`, `NoDynamicExtensions`. Types that appear only inside public signatures, such as the concrete diff --git a/crates/solverforge-solver/benches/selector_cursor_gate/construction_cases.rs b/crates/solverforge-solver/benches/selector_cursor_gate/construction_cases.rs index a91d3af1..81209d65 100644 --- a/crates/solverforge-solver/benches/selector_cursor_gate/construction_cases.rs +++ b/crates/solverforge-solver/benches/selector_cursor_gate/construction_cases.rs @@ -41,6 +41,7 @@ fn construction_full() { mix(&mut order_hash, value); assert!(placement.release_candidate(candidate_id)); } + placements.recycle(placement); } emit("construction_full", 8_192, || { let mut count = 0; @@ -57,6 +58,7 @@ fn construction_full() { assert!(placement.release_candidate(candidate_id)); count += 1; } + placements.recycle(placement); } } (count, order_hash, identity) diff --git a/crates/solverforge-solver/src/bench/construction.rs b/crates/solverforge-solver/src/bench/construction.rs index b4ccff56..1be6c7ce 100644 --- a/crates/solverforge-solver/src/bench/construction.rs +++ b/crates/solverforge-solver/src/bench/construction.rs @@ -73,6 +73,12 @@ where .next_placement(score_director, |_| false, || false) .map(|placement| CompiledScalarPlacement { placement }) } + + /// Hands a finished placement back, as a construction phase does after + /// each step, so the next placement reuses its candidate buffers. + pub fn recycle(&mut self, placement: CompiledScalarPlacement) { + self.cursor.recycle(placement.placement); + } } impl Debug for CompiledScalarPlacementCursor<'_, S> { diff --git a/crates/solverforge-solver/src/builder/context/scalar/variable.rs b/crates/solverforge-solver/src/builder/context/scalar/variable.rs index fd135dec..5a8e42f8 100644 --- a/crates/solverforge-solver/src/builder/context/scalar/variable.rs +++ b/crates/solverforge-solver/src/builder/context/scalar/variable.rs @@ -214,39 +214,73 @@ impl ScalarVariableSlot { entity_index: usize, value_candidate_limit: Option, ) -> Vec { - if let Some(provider) = self.candidate_values { - let values = provider(solution, entity_index, self.variable_index); - return match value_candidate_limit { - Some(limit) => values.iter().copied().take(limit).collect(), - None => values.to_vec(), - }; + let mut values = Vec::new(); + self.extend_candidate_values_for_entity( + solution, + entity_index, + value_candidate_limit, + &mut values, + ); + values + } + + /// Appends [`Self::candidate_values_for_entity`] to `values` in one bulk + /// copy, so a caller refilling its own buffer allocates at most once. + pub(crate) fn extend_candidate_values_for_entity( + &self, + solution: &S, + entity_index: usize, + value_candidate_limit: Option, + values: &mut Vec, + ) { + match self.candidate_values_of(solution, entity_index, value_candidate_limit) { + CandidateValues::Slice(slice) => values.extend_from_slice(slice), + CandidateValues::Range(range) => values.extend(range), + } + } + + /// Visits [`Self::candidate_values_for_entity`] in order without + /// collecting them. + pub(crate) fn visit_candidate_values_for_entity( + &self, + solution: &S, + entity_index: usize, + value_candidate_limit: Option, + visit: &mut dyn FnMut(usize), + ) { + match self.candidate_values_of(solution, entity_index, value_candidate_limit) { + CandidateValues::Slice(slice) => slice.iter().for_each(|&value| visit(value)), + CandidateValues::Range(range) => range.for_each(visit), } + } + fn candidate_values_of<'a>( + &self, + solution: &'a S, + entity_index: usize, + value_candidate_limit: Option, + ) -> CandidateValues<'a> { + let limit = value_candidate_limit.unwrap_or(usize::MAX); + let truncate = |values: &'a [usize]| &values[..limit.min(values.len())]; + if let Some(provider) = self.candidate_values { + return CandidateValues::Slice(truncate(provider( + solution, + entity_index, + self.variable_index, + ))); + } match self.value_source { - ValueSource::Empty => Vec::new(), + ValueSource::Empty => CandidateValues::Range(0..0), ValueSource::CountableRange { from, to } => { - let end = value_candidate_limit - .map(|limit| from.saturating_add(limit).min(to)) - .unwrap_or(to); - (from..end).collect() + CandidateValues::Range(from..from.saturating_add(limit).min(to)) } ValueSource::SolutionCount { count_fn, provider_index, - } => { - let count = count_fn(solution, provider_index); - let end = value_candidate_limit - .map(|limit| limit.min(count)) - .unwrap_or(count); - (0..end).collect() - } - ValueSource::EntitySlice { values_for_entity } => { - let values = values_for_entity(solution, entity_index, self.variable_index); - match value_candidate_limit { - Some(limit) => values.iter().copied().take(limit).collect(), - None => values.to_vec(), - } - } + } => CandidateValues::Range(0..limit.min(count_fn(solution, provider_index))), + ValueSource::EntitySlice { values_for_entity } => CandidateValues::Slice(truncate( + values_for_entity(solution, entity_index, self.variable_index), + )), } } @@ -401,3 +435,9 @@ impl fmt::Debug for ScalarVariableSlot { .finish() } } + +/// The candidate values of one entity, already cut to the candidate limit. +enum CandidateValues<'a> { + Slice(&'a [usize]), + Range(std::ops::Range), +} diff --git a/crates/solverforge-solver/src/builder/context/scalar_access.rs b/crates/solverforge-solver/src/builder/context/scalar_access.rs index 78e3920c..b38d9e44 100644 --- a/crates/solverforge-solver/src/builder/context/scalar_access.rs +++ b/crates/solverforge-solver/src/builder/context/scalar_access.rs @@ -229,6 +229,28 @@ impl RuntimeScalarSlot { } } + /// Appends the candidates [`Self::visit_candidate_values`] visits to + /// `values` in one bulk copy, so a caller refilling its own buffer + /// allocates at most once. + pub(crate) fn extend_candidate_values( + &self, + solution: &S, + entity_index: usize, + limit: Option, + values: &mut Vec, + ) { + match self { + Self::Static(slot) => { + slot.extend_candidate_values_for_entity(solution, entity_index, limit, values); + } + Self::Dynamic(slot) => { + let candidates = slot.candidate_values(solution, entity_index); + let limit = limit.unwrap_or(usize::MAX).min(candidates.len()); + values.extend_from_slice(&candidates[..limit]); + } + } + } + /// Visits ordinary candidates in their declared source order. `limit` /// limits source consumption, never a post-hoc result set, so lazy dynamic /// bindings and static value ranges share one ordering contract. @@ -242,9 +264,7 @@ impl RuntimeScalarSlot { let limit = limit.unwrap_or(usize::MAX); match self { Self::Static(slot) => { - for value in slot.candidate_values_for_entity(solution, entity_index, Some(limit)) { - visit(value); - } + slot.visit_candidate_values_for_entity(solution, entity_index, Some(limit), visit); } Self::Dynamic(slot) => { for &value in slot diff --git a/crates/solverforge-solver/src/heuristic/selector/move_selector/arena.rs b/crates/solverforge-solver/src/heuristic/selector/move_selector/arena.rs index 744ab6ea..74da1e76 100644 --- a/crates/solverforge-solver/src/heuristic/selector/move_selector/arena.rs +++ b/crates/solverforge-solver/src/heuristic/selector/move_selector/arena.rs @@ -28,7 +28,6 @@ where /// Drops every stored candidate while retaining both backing /// capacities, so a reused store does not re-grow its vectors. - #[cfg(test)] pub(crate) fn reset(&mut self) { self.positions.clear(); self.live.clear(); diff --git a/crates/solverforge-solver/src/phase/construction/phase/phase_type.rs b/crates/solverforge-solver/src/phase/construction/phase/phase_type.rs index 44c79f64..4fb18b84 100644 --- a/crates/solverforge-solver/src/phase/construction/phase/phase_type.rs +++ b/crates/solverforge-solver/src/phase/construction/phase/phase_type.rs @@ -162,6 +162,7 @@ where ); step_scope.complete(); + placement_cursor.recycle(placement); } let previous_best_score = phase_scope.solver_scope().best_score().copied(); diff --git a/crates/solverforge-solver/src/phase/construction/placer.rs b/crates/solverforge-solver/src/phase/construction/placer.rs index 8ce0acfc..2fab5c66 100644 --- a/crates/solverforge-solver/src/phase/construction/placer.rs +++ b/crates/solverforge-solver/src/phase/construction/placer.rs @@ -159,6 +159,11 @@ where &mut self.candidates } + /// Ends the placement, handing back its candidate cursor. + pub(crate) fn into_candidates(self) -> C { + self.candidates + } + pub(crate) fn take_move(&mut self, candidate_id: CandidateId) -> M { self.candidates.take_candidate(candidate_id) } @@ -234,6 +239,13 @@ where D: Director, IsCompleted: FnMut(&Placement) -> bool, ShouldStop: FnMut() -> bool; + + /// Takes back a placement the phase has finished with, so a cursor that + /// reuses its candidate buffers across placements can reclaim them. A + /// cursor that keeps no buffers drops it. + fn recycle(&mut self, placement: Placement) { + drop(placement); + } } pub(crate) trait EntityPlacer: Send + Debug diff --git a/crates/solverforge-solver/src/phase/construction/runtime_slots/global.rs b/crates/solverforge-solver/src/phase/construction/runtime_slots/global.rs index b63df633..4924cef5 100644 --- a/crates/solverforge-solver/src/phase/construction/runtime_slots/global.rs +++ b/crates/solverforge-solver/src/phase/construction/runtime_slots/global.rs @@ -191,7 +191,7 @@ fn candidate_values( limit: Option, ) -> Vec { let mut values = Vec::new(); - slot.visit_candidate_values(solution, entity, limit, &mut |value| values.push(value)); + slot.extend_candidate_values(solution, entity, limit, &mut values); values } diff --git a/crates/solverforge-solver/src/phase/construction/runtime_slots/placement.rs b/crates/solverforge-solver/src/phase/construction/runtime_slots/placement.rs index 39f730bb..9deadc64 100644 --- a/crates/solverforge-solver/src/phase/construction/runtime_slots/placement.rs +++ b/crates/solverforge-solver/src/phase/construction/runtime_slots/placement.rs @@ -208,33 +208,24 @@ where indices } + /// Refills `values` with the entity's candidate values in placement + /// order. Ascending-key order sorts stably, so equal keys keep the order + /// the slot offered them in. fn ordered_values( &self, slot: &RuntimeScalarSlot, solution: &S, entity_index: usize, - ) -> Vec { - let mut values = Vec::new(); - slot.visit_candidate_values( - solution, - entity_index, - self.value_candidate_limit, - &mut |value| values.push(value), - ); + values: &mut Vec, + ) { + values.clear(); + slot.extend_candidate_values(solution, entity_index, self.value_candidate_limit, values); if self.value_order == ValueOrder::AscendingKey { - let mut indexed = values.into_iter().enumerate().collect::>(); - indexed.sort_by(|(left_order, left_value), (right_order, right_value)| { - let left_key = slot - .construction_value_order_key(solution, entity_index, *left_value) - .expect("validated runtime scalar queue construction must provide a value order key"); - let right_key = slot - .construction_value_order_key(solution, entity_index, *right_value) - .expect("validated runtime scalar queue construction must provide a value order key"); - left_key.cmp(&right_key).then(left_order.cmp(right_order)) + values.sort_by_cached_key(|value| { + slot.construction_value_order_key(solution, entity_index, *value) + .expect("validated runtime scalar queue construction must provide a value order key") }); - return indexed.into_iter().map(|(_, value)| value).collect(); } - values } } @@ -243,7 +234,8 @@ where S: PlanningSolution, { store: CandidateStore>, - values: std::vec::IntoIter, + values: Vec, + next_value: usize, slot: RuntimeScalarSlot, entity_index: usize, } @@ -253,7 +245,8 @@ where S: PlanningSolution, { fn next_candidate(&mut self, _solution: &S) -> Option { - let value = self.values.next()?; + let value = *self.values.get(self.next_value)?; + self.next_value += 1; Some(self.store.push(RuntimeScalarConstructionMove::new( self.slot.clone(), self.entity_index, @@ -285,6 +278,10 @@ where next_slot_index: usize, active_slot_index: Option, entity_indices: std::vec::IntoIter, + /// The candidate store and value buffer of the last recycled placement, + /// reused by the next one so placements do not reallocate them. + spare_store: CandidateStore>, + spare_values: Vec, } impl EntityPlacer> for RuntimeScalarConstructionPlacer @@ -303,6 +300,8 @@ where next_slot_index: 0, active_slot_index: None, entity_indices: Vec::new().into_iter(), + spare_store: CandidateStore::new(), + spare_values: Vec::new(), } } } @@ -351,17 +350,21 @@ where if frozen.slot.current_value(solution, entity_index).is_some() { continue; } - let values = self - .placer - .ordered_values(&frozen.slot, solution, entity_index); - if values.is_empty() { + self.placer.ordered_values( + &frozen.slot, + solution, + entity_index, + &mut self.spare_values, + ); + if self.spare_values.is_empty() { continue; } let placement = Placement::new( EntityReference::new(frozen.slot.descriptor_index(), entity_index), RuntimeScalarCandidateCursor { - store: CandidateStore::with_capacity(values.len()), - values: values.into_iter(), + store: std::mem::take(&mut self.spare_store), + values: std::mem::take(&mut self.spare_values), + next_value: 0, slot: frozen.slot.clone(), entity_index, }, @@ -374,7 +377,20 @@ where if !is_completed(&placement) { return Some(placement); } + self.recycle(placement); } None } + + fn recycle( + &mut self, + placement: Placement, Self::CandidateCursor>, + ) { + let RuntimeScalarCandidateCursor { + mut store, values, .. + } = placement.into_candidates(); + store.reset(); + self.spare_store = store; + self.spare_values = values; + } } diff --git a/crates/solverforge-solver/tests/construction_buffer_reuse.rs b/crates/solverforge-solver/tests/construction_buffer_reuse.rs new file mode 100644 index 00000000..b8908f18 --- /dev/null +++ b/crates/solverforge-solver/tests/construction_buffer_reuse.rs @@ -0,0 +1,84 @@ +//! A construction placement cursor reuses the buffers of a placement handed +//! back to it, so after the first placement the next ones allocate no +//! candidate store or value buffer. +//! +//! This is its own test binary because it counts the process's allocations. + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use solverforge_config::{ + ConstructionHeuristicConfig, ConstructionHeuristicType, PhaseConfig, SolverConfig, +}; +use solverforge_solver::__bench::CompiledRuntimeRunner; + +mod support; + +struct CountingAllocator; + +static ALLOCATIONS: AtomicUsize = AtomicUsize::new(0); + +// SAFETY: every call forwards to the system allocator unchanged. +unsafe impl GlobalAlloc for CountingAllocator { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOCATIONS.fetch_add(1, Ordering::Relaxed); + unsafe { System.alloc(layout) } + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } + + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOCATIONS.fetch_add(1, Ordering::Relaxed); + unsafe { System.realloc(ptr, layout, new_size) } + } +} + +#[global_allocator] +static ALLOCATOR: CountingAllocator = CountingAllocator; + +#[test] +fn a_recycled_placement_lends_its_buffers_to_the_next() { + let config = SolverConfig { + random_seed: Some(7), + phases: vec![PhaseConfig::ConstructionHeuristic( + ConstructionHeuristicConfig { + construction_heuristic_type: ConstructionHeuristicType::FirstFit, + ..ConstructionHeuristicConfig::default() + }, + )], + ..SolverConfig::default() + }; + let placer = CompiledRuntimeRunner::compile(&config, support::context()) + .expect("the runner compiles") + .into_scalar_construction_placer() + .expect("the first phase is a scalar construction"); + let director = support::director(support::unassigned(4)); + let mut placements = placer.open_cursor(&director); + + let mut placement_allocations = Vec::with_capacity(8); + let mut values = Vec::with_capacity(8); + loop { + let before = ALLOCATIONS.load(Ordering::Relaxed); + let Some(mut placement) = placements.next_placement(&director) else { + break; + }; + values.clear(); + while let Some(id) = placement.next_candidate(director.working_solution()) { + values.push(placement.candidate_assignment(id).expect("live").1); + assert!(placement.release_candidate(id)); + } + placement_allocations.push(ALLOCATIONS.load(Ordering::Relaxed) - before); + assert_eq!(values, [0, 1, 2, 3]); + placements.recycle(placement); + } + + // Every task is a placement, offered every worker in order. The first + // placement allocates the slot's entity order and the buffers; each later + // one allocates only its construction target, reusing the recycled + // candidate store and value buffer. + assert_eq!(placement_allocations.len(), 4); + assert!(placement_allocations[0] > 1, "{placement_allocations:?}"); + assert_eq!(placement_allocations[1..], [1, 1, 1]); +}