From 4e043364fd24f892216d90d4dff27e43f323a4d5 Mon Sep 17 00:00:00 2001
From: Anders Olsson <anders.e.olsson@gmail.com>
Date: Thu, 27 Aug 2026 17:50:39 +0200
Subject: [PATCH] perf: stop cloning inference inputs in OwnedGame and
 ingestion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 src/game.rs    | 21 ++++++++++-----------
 src/history.rs | 38 ++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/game.rs b/src/game.rs
index 938588e..bc69584 100644
--- a/src/game.rs
+++ b/src/game.rs
@@ -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,
         }
diff --git a/src/history.rs b/src/history.rs
index 746ca84..912414f 100644
--- a/src/history.rs
+++ b/src/history.rs
@@ -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(&times, 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();