3e69241c64
User goal clarified: the wildly fluctuating FPS/ms IS the target. Root cause is per-frame throwaway allocations triggering periodic gen2 GC pauses = the spikes. Plan takes only the easy, bit-identical, non-faithfulness-sensitive buffer-reuse sites from the 54-site audit (animation pose buffers, particle draw-lists/iterator, interior partition, trivial per-frame HashSets); defers the two risky sites (EnvCell settled-camera gate, physics Transition pooling). Gate = the existing frame profiler before/after (alloc_kb down, cpu_ms max/p99 tighten) + identical visuals. Loading/pak work parked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
41 lines
5.2 KiB
Markdown
41 lines
5.2 KiB
Markdown
# MP-Alloc Safe Batch — kill the per-frame garbage that spikes frame time
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:subagent-driven-development. Steps use checkbox syntax.
|
||
|
||
**Goal:** Eliminate the largest *easy, bit-identical* per-frame allocations in the render/update hot path, so the GC-pause spikes that make FPS/ms fluctuate wildly go away. Target: the profiler's `alloc_kb/frame` drops sharply and cpu_ms p99/max tighten toward the p50, with **zero visual or gameplay change**.
|
||
|
||
**Why this batch:** MP0 measured 1.5–3 MB/frame allocated in steady-state dense-town, driving gen2 GC ~1–2/s → 20–87 ms frame spikes (the wild fluctuation the user sees on the counter). A 54-site allocation audit ranked the sources. This batch takes ONLY the sites that are (a) high-ranked, (b) pure buffer/list reuse — identical values, just not re-allocated — and (c) NOT faithfulness-sensitive. The two risky sites (EnvCellRenderer.PrepareRenderBatches settled-camera dirty-gate; PhysicsEngine `Transition` pooling) are DEFERRED to their own careful slices — do not touch them here.
|
||
|
||
**Binding rules:**
|
||
- **Bit-identical output.** Every change reuses a buffer/list that previously was `new`'d each frame. The produced values must be identical — reuse means `.Clear()` + refill or overwrite-in-place, never a semantic change. If a fix would change any drawn value, STOP and report.
|
||
- **Existing test suite stays green** (4120 tests). No test may be modified.
|
||
- **Thread-safety preserved.** Several of these run on the render thread only; some (particles) may be touched from multiple passes per frame — keep the existing access pattern; a per-owner reusable field is fine when the owner is single-threaded, otherwise report.
|
||
- **No new pooling framework.** Use plain reusable fields (`private readonly List<T> _scratch = new()`) cleared per use, or per-entity cached arrays. No ArrayPool ceremony unless a site obviously needs it.
|
||
- Measure with the existing `[frame-prof]` meter (env `ACDREAM_FRAME_PROF=1`) — before/after `alloc_kb` and cpu_ms p99/max.
|
||
|
||
---
|
||
|
||
## Sites (each its own commit; skip any that can't be made bit-identical and report)
|
||
|
||
### Task 1 — Animation pose buffer reuse (highest effort:payoff)
|
||
**Files:** `src/AcDream.Core/Physics/AnimationSequencer.cs` (~747/770), `src/AcDream.App/Rendering/GameWindow.cs` (~10723).
|
||
Every animated entity (incl. idle NPCs on a breathe cycle) allocates a fresh `PartTransform[partCount]` in Advance AND a fresh `List<MeshRef>(partCount)` reassigned to `entity.MeshRefs` each frame (old → garbage). Fix: cache a `PartTransform[]` and a reusable `List<MeshRef>` per entity (sized to partCount), overwrite/clear in place, stop reassigning `entity.MeshRefs` to a new list. Verify: pose values identical (existing animation/conformance tests green); no consumer relies on `MeshRefs` being a fresh list each frame (check for cached references / identity comparisons before reusing).
|
||
|
||
### Task 2 — Particle draw-list + accumulator reuse
|
||
**Files:** `src/AcDream.App/Rendering/ParticleRenderer.cs` (~147/181), `src/AcDream.Core/Vfx/ParticleSystem.cs` (~161 `EnumerateLive`).
|
||
`Draw` is called many times/frame (sky pre/post, scene passes, per visible cell, dynamics, unattached), each `new`ing `List<ParticleDraw>` + `List<ParticleInstance>` + a yield-iterator. Fix: reuse `_drawList`/`_run` fields with `.Clear()`; replace the `EnumerateLive()` yield-iterator with a struct enumerator or index walk (no per-call iterator allocation). Keep the per-cell filtering semantics identical. (The deeper O(cells×particles) re-walk is a SEPARATE later structural fix — this task is only the buffer/iterator allocations.)
|
||
|
||
### Task 3 — Interior entity partition pooling
|
||
**File:** `src/AcDream.App/Rendering/InteriorEntityPartition.cs` (~46).
|
||
`Partition` news a `Result` (Dict + 2 Lists) + a `List<WorldEntity>` per visible cell every frame. Fix: make `Partition` reuse a cleared-in-place `Result` and pooled per-cell lists owned by the partitioner. Identical partitioning output.
|
||
|
||
### Task 4 — Trivial per-frame HashSet / small-collection reuse
|
||
**Files:** `src/AcDream.App/Rendering/GameWindow.cs` (~9092 `animatedIds`, note the near-duplicate at the DrawContext capture ~9314 — dedupe if they're the same intent), `src/AcDream.App/Rendering/RetailPViewRenderer.cs` (~113 `drawableCells`), `src/AcDream.App/Rendering/IndoorDrawPlan.cs` (~20 ShellPass lists), and `DrainCompletions` (per the audit, ~one-liner).
|
||
Each rebuilds a fresh `HashSet<uint>`/`List<>` from keys every frame. Fix: reusable cleared-in-place fields. These are one-liners; batch them together in this one commit. If the two `animatedIds` sites are the same set built twice, build once and share.
|
||
|
||
---
|
||
|
||
## Verification (the gate)
|
||
- After each task: `dotnet build` + full `dotnet test` green.
|
||
- After the batch: `dotnet build -c Release`, then a live launch with `ACDREAM_FRAME_PROF=1`. The coordinator + user compare the `[frame-prof]` line in a dense town BEFORE (baseline: alloc_kb p50 ~1600–3000, cpu_ms max 20–87) vs AFTER. Success = alloc_kb p50 down materially AND cpu_ms max/p99 tightened toward p50, with the user confirming the scene looks and plays identically.
|
||
- If a site's numbers don't move or a fix risks a visual change, report it — we escalate to the dotnet-trace profile to re-rank rather than guessing.
|