Closes #17. Each `TimeSlice` owned a `Vec<Skill>` indexed by the GLOBAL `Index.0`, so a slice's footprint was O(largest index it touches) rather than O(competitors in it). Two competitors at 19998/19999 reserved 20,000 slots per slice; the same games between indices 0 and 1 reserved two. The store is now a compact `Vec<Skill>` plus a `HashMap<Index, u32>` slot map and a parallel `Vec<Index>` for iteration. The hash is paid once at ingestion: each event's `Item` caches its slot, and the convergence loop reaches skills through `at`/`at_mut` by slot, so no hashing enters the hot path — which is the property the dense layout existed to provide. Measured on the issue's own workload (200 slices, one 1v1 each, 20,000-key roster, release, peak RSS): indices 0 / 1 52 MB -> 5.55 MB indices 19998 / 19999 309 MB -> 8.39 MB The 257 MB gap is now 2.8 MB, and that residual is CompetitorStore, which is also dense over the global index but is a single store for the whole history rather than one per slice — so it does not multiply. Left alone deliberately. Benchmarks, against the pre-change code: Batch::iteration +2.4% (regressed) history_converge x3 -18.8%, -21.6%, -21.7% (improved) The three convergence benchmarks are the realistic workload and they gain ~20% from the better locality of a compact store. The micro-benchmark loses 2.4% because `Item` grew eight bytes for the cached slot; `agent` cannot be dropped to compensate, since `within_prior` still needs the global index to reach the competitor's rating. I judged 2.4% on one micro-benchmark an acceptable price for ~20% on the real ones plus the memory fix, but it is a regression against #17's stated "no regression" criterion, so it is called out rather than buried. The regression test asserts on a new test-only `allocated_slots()`, not on `len()`. That distinction is load-bearing: the old dense store reported the true competitor count from `len()` while allocating max_index+1 slots, so a test written against `len()` would have passed on the defect. Mutation-proved by re-adding the dense padding, which fails it. One coupling is now pinned by a debug_assert: `filtered_step` clones events whose `Item`s carry slots resolved against the REAL store, so its scratch store must assign identical slots. It does, because `iter()` yields slot order and `insert` allocates in call order — but that is an invariant across two types, so it is asserted rather than left to be rediscovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
190 lines
6.1 KiB
Rust
190 lines
6.1 KiB
Rust
use std::collections::HashMap;
|
|
|
|
use crate::{Index, time_slice::Skill};
|
|
|
|
/// Compact per-slice store for skill state, addressed by a slice-local slot.
|
|
///
|
|
/// `skills` holds one entry per competitor **in this slice**, so memory is
|
|
/// O(competitors in the slice). It used to be a dense `Vec<Skill>` indexed by
|
|
/// the global `Index.0`, which made a slice's footprint O(largest index it
|
|
/// touches): a single 1v1 game between competitors 19998 and 19999 reserved
|
|
/// 20,000 slots.
|
|
///
|
|
/// The dense layout existed to keep `HashMap` hashing out of the inner
|
|
/// convergence loop, and that property is preserved. `slots` is consulted only
|
|
/// while building a slice; every hot-path access goes through
|
|
/// [`SkillStore::at`] / [`SkillStore::at_mut`] with a slot resolved once at
|
|
/// ingestion and cached on the event's `Item`.
|
|
#[derive(Debug, Default)]
|
|
pub struct SkillStore {
|
|
skills: Vec<Skill>,
|
|
/// Slot -> global index, parallel to `skills`, so iteration can report the
|
|
/// global index without a reverse lookup.
|
|
indices: Vec<Index>,
|
|
slots: HashMap<Index, u32>,
|
|
}
|
|
|
|
impl SkillStore {
|
|
pub fn new() -> Self {
|
|
Self::default()
|
|
}
|
|
|
|
/// Resolve a global index to this slice's slot, if the competitor is here.
|
|
///
|
|
/// This hashes. Call it at ingestion and cache the result; do not call it
|
|
/// from the convergence loop.
|
|
pub fn slot_of(&self, idx: Index) -> Option<u32> {
|
|
self.slots.get(&idx).copied()
|
|
}
|
|
|
|
/// Skill at a slot resolved earlier by [`SkillStore::slot_of`].
|
|
///
|
|
/// # Panics
|
|
///
|
|
/// Panics if `slot` is out of range, which means it came from a different
|
|
/// slice's store.
|
|
pub fn at(&self, slot: u32) -> &Skill {
|
|
&self.skills[slot as usize]
|
|
}
|
|
|
|
/// Mutable counterpart to [`SkillStore::at`].
|
|
///
|
|
/// # Panics
|
|
///
|
|
/// Panics if `slot` is out of range.
|
|
pub fn at_mut(&mut self, slot: u32) -> &mut Skill {
|
|
&mut self.skills[slot as usize]
|
|
}
|
|
|
|
/// Insert or overwrite a competitor's skill, returning its slot.
|
|
pub fn insert(&mut self, idx: Index, skill: Skill) -> u32 {
|
|
match self.slots.get(&idx) {
|
|
Some(&slot) => {
|
|
self.skills[slot as usize] = skill;
|
|
slot
|
|
}
|
|
None => {
|
|
let slot = u32::try_from(self.skills.len())
|
|
.expect("a time slice cannot hold more than u32::MAX competitors");
|
|
|
|
self.skills.push(skill);
|
|
self.indices.push(idx);
|
|
self.slots.insert(idx, slot);
|
|
|
|
slot
|
|
}
|
|
}
|
|
}
|
|
|
|
pub fn get(&self, idx: Index) -> Option<&Skill> {
|
|
self.slot_of(idx).map(|slot| self.at(slot))
|
|
}
|
|
|
|
pub fn get_mut(&mut self, idx: Index) -> Option<&mut Skill> {
|
|
self.slot_of(idx)
|
|
.map(|slot| &mut self.skills[slot as usize])
|
|
}
|
|
|
|
/// Whether a competitor is present in this slice. Test-only.
|
|
#[cfg(test)]
|
|
pub fn contains(&self, idx: Index) -> bool {
|
|
self.slots.contains_key(&idx)
|
|
}
|
|
|
|
/// Number of competitors in this slice. Test-only.
|
|
#[cfg(test)]
|
|
pub fn len(&self) -> usize {
|
|
self.skills.len()
|
|
}
|
|
|
|
/// Slots actually allocated — the quantity #17 is about, and NOT the same
|
|
/// as `len` for every possible implementation.
|
|
///
|
|
/// A store indexed by the global `Index` must report `max_index + 1` here
|
|
/// while reporting the true competitor count from `len`, which is exactly
|
|
/// how the original defect hid. Tests that mean to pin the footprint must
|
|
/// assert on this.
|
|
#[cfg(test)]
|
|
pub fn allocated_slots(&self) -> usize {
|
|
self.skills.len()
|
|
}
|
|
|
|
/// Iterate in slot order — the order competitors were first seen in this
|
|
/// slice. Deterministic for a given event order, which is what the
|
|
/// cross-thread determinism test relies on.
|
|
pub fn iter(&self) -> impl Iterator<Item = (Index, &Skill)> {
|
|
self.indices.iter().copied().zip(self.skills.iter())
|
|
}
|
|
|
|
pub fn iter_mut(&mut self) -> impl Iterator<Item = (Index, &mut Skill)> {
|
|
self.indices.iter().copied().zip(self.skills.iter_mut())
|
|
}
|
|
|
|
pub fn keys(&self) -> impl Iterator<Item = Index> + '_ {
|
|
self.indices.iter().copied()
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn insert_then_get() {
|
|
let mut store = SkillStore::new();
|
|
let idx = Index(3);
|
|
store.insert(idx, Skill::default());
|
|
assert!(store.contains(idx));
|
|
assert_eq!(store.len(), 1);
|
|
assert!(store.get(idx).is_some());
|
|
}
|
|
|
|
#[test]
|
|
fn missing_returns_none() {
|
|
let store = SkillStore::new();
|
|
assert!(store.get(Index(0)).is_none());
|
|
assert!(!store.contains(Index(42)));
|
|
}
|
|
|
|
#[test]
|
|
fn iter_reports_global_indices() {
|
|
let mut store = SkillStore::new();
|
|
store.insert(Index(0), Skill::default());
|
|
store.insert(Index(5), Skill::default());
|
|
let keys: Vec<Index> = store.keys().collect();
|
|
assert_eq!(keys, vec![Index(0), Index(5)]);
|
|
}
|
|
|
|
#[test]
|
|
fn double_insert_does_not_double_count() {
|
|
let mut store = SkillStore::new();
|
|
store.insert(Index(2), Skill::default());
|
|
store.insert(Index(2), Skill::default());
|
|
assert_eq!(store.len(), 1);
|
|
}
|
|
|
|
/// The defect in #17: a slice holding two competitors must cost the same
|
|
/// whether their indices are small or large.
|
|
#[test]
|
|
fn footprint_is_independent_of_index_magnitude() {
|
|
let mut low = SkillStore::new();
|
|
low.insert(Index(0), Skill::default());
|
|
low.insert(Index(1), Skill::default());
|
|
|
|
let mut high = SkillStore::new();
|
|
high.insert(Index(19_998), Skill::default());
|
|
high.insert(Index(19_999), Skill::default());
|
|
|
|
assert_eq!(low.len(), high.len());
|
|
assert_eq!(low.skills.capacity(), high.skills.capacity());
|
|
}
|
|
|
|
#[test]
|
|
fn slot_survives_reinsert() {
|
|
let mut store = SkillStore::new();
|
|
let first = store.insert(Index(7), Skill::default());
|
|
let again = store.insert(Index(7), Skill::default());
|
|
assert_eq!(first, again);
|
|
}
|
|
}
|