perf: stop cloning inference inputs in OwnedGame and ingestion
The last two items on #23. `OwnedGame::new` and `new_scored` cloned the whole team structure to hand one copy to `Game` and keep another. But `Game` takes the teams by value and is dropped at the end of the constructor, so the vec can simply be taken back out of it — the clone existed only because nobody looked at the lifetime. `add_events_with_prior` deep-cloned each event's composition, results and weights when chunking events into per-timestamp groups. Nothing reads those three after the chunking loop (the agent-collection pass and the tie pre-check both run before it), so the elements are now moved out with `mem::take`. That soundness argument rests entirely on `o` being a permutation: visiting an index twice would take an already-emptied vec and silently produce an event with no teams rather than failing. Since that would be invisible, there is now a debug_assert checking the permutation property directly, next to the comment explaining why the code depends on it. Verified on 1.85.0 as well as the local toolchain — an MSRV break in this change would otherwise only surface in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
This commit is contained in:
+10
-11
@@ -107,16 +107,13 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
convergence: crate::ConvergenceOptions,
|
||||
) -> Self {
|
||||
let mut arena = ScratchArena::new();
|
||||
let g = Game::ranked_with_arena(
|
||||
teams.clone(),
|
||||
&result,
|
||||
&weights,
|
||||
p_draw,
|
||||
convergence,
|
||||
&mut arena,
|
||||
);
|
||||
|
||||
// `Game` takes the teams by value and is dropped here, so take the vec
|
||||
// back out of it rather than handing it a clone.
|
||||
let g = Game::ranked_with_arena(teams, &result, &weights, p_draw, convergence, &mut arena);
|
||||
|
||||
Self {
|
||||
teams,
|
||||
teams: g.teams,
|
||||
likelihoods: g.likelihoods,
|
||||
log_evidence: g.log_evidence,
|
||||
}
|
||||
@@ -130,16 +127,18 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
convergence: crate::ConvergenceOptions,
|
||||
) -> Self {
|
||||
let mut arena = ScratchArena::new();
|
||||
|
||||
let g = Game::scored_with_arena(
|
||||
teams.clone(),
|
||||
teams,
|
||||
&scores,
|
||||
&weights,
|
||||
score_sigma,
|
||||
convergence,
|
||||
&mut arena,
|
||||
);
|
||||
|
||||
Self {
|
||||
teams,
|
||||
teams: g.teams,
|
||||
likelihoods: g.likelihoods,
|
||||
log_evidence: g.log_evidence,
|
||||
}
|
||||
|
||||
+28
-10
@@ -650,10 +650,10 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O, K> {
|
||||
pub(crate) fn add_events_with_prior(
|
||||
&mut self,
|
||||
composition: Vec<Vec<Vec<Index>>>,
|
||||
results: Option<Vec<Vec<f64>>>,
|
||||
mut composition: Vec<Vec<Vec<Index>>>,
|
||||
mut results: Option<Vec<Vec<f64>>>,
|
||||
times: Vec<T>,
|
||||
weights: Option<Vec<Vec<Vec<f64>>>>,
|
||||
mut weights: Option<Vec<Vec<Vec<f64>>>>,
|
||||
kinds: Vec<EventKind>,
|
||||
mut priors: HashMap<Index, Rating<T, D>>,
|
||||
) -> Result<(), InferenceError> {
|
||||
@@ -743,6 +743,20 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
||||
let n = composition.len();
|
||||
let o = sort_time(×, false);
|
||||
|
||||
// The chunking loop below MOVES each event's data out of `composition`,
|
||||
// `results` and `weights` instead of cloning it. That is only sound
|
||||
// because `o` is a permutation, so every index is visited exactly once
|
||||
// — visiting one twice would silently yield an empty event rather than
|
||||
// failing.
|
||||
debug_assert!(
|
||||
{
|
||||
let mut seen = vec![false; n];
|
||||
o.iter()
|
||||
.all(|&idx| !std::mem::replace(&mut seen[idx], true))
|
||||
},
|
||||
"sort_time must return a permutation of 0..{n}"
|
||||
);
|
||||
|
||||
let mut i = 0;
|
||||
let mut k = 0;
|
||||
|
||||
@@ -779,16 +793,20 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
||||
}
|
||||
|
||||
let composition = (i..j)
|
||||
.map(|e| composition[o[e]].clone())
|
||||
.map(|e| std::mem::take(&mut composition[o[e]]))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let results = results
|
||||
.as_ref()
|
||||
.map(|results| (i..j).map(|e| results[o[e]].clone()).collect::<Vec<_>>());
|
||||
let results = results.as_mut().map(|results| {
|
||||
(i..j)
|
||||
.map(|e| std::mem::take(&mut results[o[e]]))
|
||||
.collect::<Vec<_>>()
|
||||
});
|
||||
|
||||
let weights = weights
|
||||
.as_ref()
|
||||
.map(|weights| (i..j).map(|e| weights[o[e]].clone()).collect::<Vec<_>>());
|
||||
let weights = weights.as_mut().map(|weights| {
|
||||
(i..j)
|
||||
.map(|e| std::mem::take(&mut weights[o[e]]))
|
||||
.collect::<Vec<_>>()
|
||||
});
|
||||
|
||||
let kinds_chunk: Vec<EventKind> = (i..j).map(|e| kinds[o[e]]).collect();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user