Remove PFE heartbeat allocation storms and quadratic scans
Follow-up to production dump/trace analysis tsk_5j446c4ges. Implementation started in dedicated worktree /home/mathias/Projects/worktrees/engines-pfe-hot-loops on branch perf/reduce-pfe-hot-loop-allocations from origin/develop f535261e1. First slice: offline harness and wall collision baseline/fix; do not mix navigation PR or unrelated work. The 60-second profile on image 16e2352 captured managed sampled-thread-time plus verbose GC/allocation events. It is not a kernel CPU profile, so exact CPU percentages remain a separate perf gate; allocation and runtime GC evidence is direct. Evidence - Estimated managed allocation: 8,879,046,864 bytes over 61.86s (~136.9 MiB/s). - 1,029 collections (~16.6/s): 730 gen0 and 299 gen1, no gen2; GC event durations total 7.17s, p95 15.4ms, max 33.4ms. Verbose tracing itself adds suspension overhead, so do not use its total pause number as an uninstrumented baseline. - DynelCollisionPfSystem.OnNpcTick/CheckCharacterDynelProximity is attributed ~2.63 GiB (29.6%) via compiler DisplayClass23_0 plus another ~95 MiB in player ticks. The method contains capturing Task.Run lambdas, so its display class is created on every hot-path invocation even when occupancy returns immediately. It also scans every RDB dynel and terminal per ticking NPC. - WallCollisionPfSystem.CheckCollision is attributed ~896 MiB (10.1%). DotProduct/CrossProduct allocate two float[2] arrays per call and are invoked repeatedly per wall segment on every player tick. - CorpseManagementPfSystem.OnTick snapshots Corpses and Terminals every tick (~241 MiB attributed); LINQ Where/Union, collection snapshots and stat enumeration are other major churn sources. - Managed wall-clock samples identify FightTallyPfSystem.ReconcileIsFightingMe as the leading active hotspot (3.67% inclusive / 1.56% exclusive of all sampled thread time): every character's 10-second reconciliation scans all NPCs and players, making it quadratic at playfield scale. Other active hotspots are DynelCollision, brute-force ACG triangle ray tests, Bepu overlap queries and indoor door handling. Intended outcome Reduce steady PFE CPU and allocation rate without changing collision triggers, combat tally correction, zoning, corpse lifecycle or path/LoS behavior. Priority plan / acceptance 1. Move dynel collision async work into non-capturing helpers invoked only after a confirmed enter/leave transition; prove no display-class allocation on steady no-event checks. Cache immutable dynel metadata and spatially index or cadence-gate proximity checks instead of NPC × all-dynels × frame. Preserve delayed recheck and occupancy cleanup semantics. 2. Rewrite wall math with scalar locals/value types; avoid any allocation on no-collision checks. Precompute wall segment geometry/spatial lookup if scans remain measurable. 3. Make FightTally authoritative/event-driven where hate membership changes; retain a bounded batched diagnostic reconciliation rather than all-characters × all-characters scans every 10 seconds. 4. Remove unconditional per-tick collection snapshots in corpse/terminal and related systems where concurrent enumeration or change queues are safe. Consolidate repeated stat Values/Union materialization. 5. Use synthetic counters/allocation assertions and representative live-world load. Compare allocation MiB/s, GC frequency/pause, tick p95/p99 and true Linux perf CPU samples. Preserve behavior tests for dynel enter/end-collide, teleports, player/NPC removal, fight drift correction, wall zoning, corpses and ACG LoS/pathing. Exact CPU attribution requires a future collect-linux/perf CPU profile with suitable container permissions; do not present managed wall-clock sample percentages as core utilization. Planning refinement: concurrency and ownership constraints - Treat AOEMU_PARALLEL_CELL_TICKS=true as supported even though Ithaca currently sets false. Different cells can call the same per-playfield system concurrently; each NPC is protected by its TickLock, but system-global mutable state is not implicitly single-threaded. Session handlers, Task.Run continuations, lifecycle callbacks and the heartbeat can also overlap. A production optimization may not replace a stable copy with unsynchronized mutable enumeration or reuse one shared scratch collection across workers. - State must have an explicit owner. Prefer immutable indexes atomically published on infrequent topology changes, per-character state mutated only while holding that character's TickLock, and MPSC command queues drained by the playfield heartbeat for cross-thread lifecycle/event handoff. Use ConcurrentDictionary only where weakly consistent semantics are explicitly acceptable. Disposal uses a generation/closed gate so late work cannot mutate a new or dead playfield. - Preserve stable-iteration semantics where required. Copy-on-write at terminal/dynel add/remove time is acceptable; per-frame copying is not. If a source changes during processing, use versioned publication/retry or idempotent commands rather than enumerating a List/Dictionary while another thread mutates it. - Delayed dynel work should not remain arbitrary Task.Run code touching mutable Character/Dynel objects. Prefer the existing TickLock-owned TimedAction path or a playfield-owned due-command queue carrying identities, source generation and immutable event data; re-resolve and validate at execution. Allocations on a rare enter/leave event are acceptable for this milestone; steady no-event checks are not. PRK.Client patterns to reuse selectively - PhysicsEngine: producers enqueue commands while one owner thread mutates dictionaries/lists; pose publication uses generation handles and a seqlock-style sequence. Adapt the ownership/generation pattern, not its single-client assumptions. - IndoorSurface/WorldManager: immutable scene/index snapshots are published with Volatile.Write and readers never observe an object being mutated. Use this for static/dynamic proximity and wall indexes, rebuilding only changed buckets on topology changes. - AssetCache: bounded worker ownership, explicit completion handoff and deterministic shutdown. Use analogous stale-generation rejection for deferred PFE work; do not create a second general task runtime. - InstanceBatchTableTests: warm twice, then assert GC.GetAllocatedBytesForCurrentThread()==0 for a stable frame. Apply exact-zero assertions to synchronous per-character hot paths with hand-written fakes. Add aggregate allocation/throughput tests for parallel work because current-thread counters do not see worker allocations. Proposed implementation sequence 1. Measurement and concurrency harness. Add a small reusable hot-loop allocation assertion/test fixture (not production instrumentation), deterministic parallel-cell/lifecycle stress harnesses and behavior baselines. Warm JIT/capacity before measuring. Record current per-1000-tick allocations for dynel, wall, fight and corpse paths. 2. Wall collision first: pure scalar math, then precompute immutable wall-segment geometry/spatial buckets per playfield and atomically publish. Static RDB walls never need per-tick copy/lock. Stable no-hit and hit checks must allocate exactly zero after warmup and preserve factors/zoning at segment endpoints/corners. 3. Dynel collision: build immutable spatial buckets for static RDB dynels and versioned copy-on-write buckets for dynamic terminals via explicit terminal lifecycle hooks. Query only intersecting buckets. Move occupancy to per-character state owned under TickLock and remove it atomically on character teardown. Move delayed actions to owner-tick scheduling with generation checks. Stable outside/inside/exit checks must allocate zero; transition paths preserve authored teleport chains and OnEndCollide exactly once. 4. Fight tally: audit all hate mutations and make one event-driven reverse count authoritative with atomic/owner-serialized add/remove/clear transitions. Do not maintain two independently mutable truths. Replace per-character all-world correction with bounded reconciliation/diagnostics that never writes a transient inconsistent snapshot: use mutation versions or owner-drained deltas, and process a fixed budget per tick. Stress TrackHate/Remove/Clear/death/despawn concurrently and preserve correction broadcasts. 5. Corpse/terminal lifecycle: replace unconditional dictionary snapshots/scans with playfield-owner due queues keyed by identity, deadline and generation. Cross-thread adds/removes enqueue idempotent commands; owner drains them. Keep authoritative ConcurrentDictionary lookups for validation. Do not directly enumerate mutable ordinary collections. Warmed empty/no-due OnTick must allocate zero. 6. Integrate and profile after each slice. Prefer separate reviewable commits/PRs per system if behavior changes become large. Run allocation tests, concurrent mutation/disposal stress, existing gameplay regressions and a representative local 300-player/NPC workload. Final gate compares allocation MiB/s, GC/s, tick p95/p99 and Linux perf CPU stacks. The milestone aims for near-zero allocation in these worst paths; whole-heartbeat zero allocation remains a later goal. Offline empirical reproduction harness — first deliverable Build a small standalone `PfeHotLoopHarness` (name/location should follow existing Tools/benchmark conventions after implementation inspection) that exercises the exact production hot-path components without a login/session flow. It must not start engine listeners, migrations, DB contexts, RabbitMQ, RDB loading, telemetry exporters or game-data preload. No client, credentials or AO installation required. Harness architecture - Extract dependency-light kernels/indexes where needed, then call those exact production types from both the systems and harness. Do not maintain benchmark-only copies of wall math, proximity rules or fight tally logic. System integration coverage uses hand-written no-op fakes, not Moq invocation tracking. - Build all fixture topology before measurement: synthetic cells, players/NPCs, dynels, terminals, wall segments, hate edges and corpses with deterministic seeds. Use a minimal no-op PF session only when a production signature requires one; steady-state scenarios must avoid networking or serialization side effects. - Provide named workload shapes: `smoke` for CI; `production-peak` approximating one PFE's observed world (configurable playfields/players/NPCs/dynels/walls/hate density); and focused per-system scenarios. Keep anonymized counts/shapes only—no game assets or captured player data. - Modes: serial heartbeat, AOEMU_PARALLEL_CELL_TICKS-style parallel workers with configurable DOP, concurrent topology churn, concurrent hate add/remove/clear, and cleanup racing queued/delayed work. Use barriers and deterministic operation streams so concurrency failures are reproducible. - Ownership remains explicit. One coordinator owns mutable fixture lifecycle; worker operations follow production TickLock/index/queue rules. Harness measurement code must not hide races by globally locking the scenario. Measurements and output - Warm JIT, indexes, buffers and collection capacity before the measured phase. Run in a fresh process and exclude fixture setup. Support fixed-iteration and fixed-duration runs. - Report machine-readable JSON plus concise text: elapsed and process CPU time, average utilized cores, operations/ticks per second, `GC.GetTotalAllocatedBytes(precise:true)` delta, bytes/op and MiB/s, gen0/1/2 deltas, working-set change, and tick p50/p95/p99/max from a preallocated sample buffer. Per-thread exact-zero tests continue to use `GetAllocatedBytesForCurrentThread` for synchronous kernels. - A `--wait-for-profiler`/duration mode prints PID and scenario boundaries so `perf`, `dotnet-trace collect-linux` or another external profiler can capture the isolated workload. The harness itself does not add a permanent profiling subsystem. - Persist the pre-fix baseline output as task/PR evidence, not as a brittle CI timing golden file. CI enforces semantic invariants and allocation ceilings; local/manual runs compare CPU/throughput because shared-runner timing is unstable. Required reproductions before optimization - Dynel steady outside/inside loops reproduce the DisplayClass allocation storm and NPC × dynel scan; transition frequency is separately configurable so rare-path allocations are visible but not mixed into steady-state zero-allocation assertions. - Wall no-hit scan reproduces repeated float[2] allocation across realistic segment counts. - Fight tally creates configurable hate graphs and demonstrates reconciliation scaling as character/NPC counts increase (for example N, 2N, 4N), making quadratic behavior observable. - Corpse/terminal empty, steady and due-item scenarios expose per-tick snapshot costs. - Parallel/churn modes must fail deterministically on collection corruption, duplicate/missed transition, stale-generation execution or count drift. Harness acceptance A clean local machine can reproduce the current allocation rankings in minutes with one command and no session login/infrastructure. After each fix, the same command demonstrates the delta. Stable wall and dynel no-event kernels target exactly zero allocation after warmup; fight/corpse steady paths target zero or a documented constant bound. Production-peak aggregate allocation should fall by at least the measured contributions of the changed paths, with preserved outputs and no race failures. The quick CI scenario completes in a bounded time; the full scenario remains opt-in. Implementation milestone 1 - Added Tools/PfeHotLoopHarness on the task branch. It references production engine types directly and starts no infrastructure. Current scenarios: wall and dynel, smoke/production-peak, serial/configurable parallelism, deterministic synthetic topology, profiler attach delay, JSON/text output, total allocation, GC deltas, process CPU/average cores, working-set delta and preallocated tick percentiles. - Dynel pre-fix baseline is now empirical: smoke 51,200 candidate checks allocate 4,096,936 bytes (80.018 B/candidate); production shape 768,000 checks with DOP 8 allocates 61,445,744 bytes (80.007 B/candidate), 4 gen0 + 1 gen1, in 17ms. This directly reproduces the 80-byte compiler display-class allocation on every steady outside candidate without login/DB/RabbitMQ/RDB assets. Serial warmup initializes the existing non-thread-safe occupancy map before parallel read stress; the harness does not pretend that makes concurrent mutation safe. - Implemented immutable WallCollisionIndex: copies RDB endpoint values once, skips degenerate segments, precomputes deltas/inverse lengths, supports lock-free concurrent readers and never exposes mutable RDB objects. WallCollisionPfSystem atomically publishes the index, uses scalar position math and an allocation-free nullable-free Try API; destination behavior remains on rare hit path. - Wall tests cover near hit/destination/factor, strict threshold/endpoints, source mutation isolation, 100,000 deterministic concurrent reads and exact zero bytes over 10,000 warmed 512-segment no-hit scans. Five focused tests pass. PFE/harness Release builds with no C# warnings (host SDK emits known package locale normalization warnings). - Post-fix wall harness: smoke 3,200 checks, no GC, 0.295 B/op aggregate harness overhead; production shape 480,000 checks/DOP8, no GC, 20,408 total bytes (0.043 B/op) from harness scheduling/samples. Exact kernel assertion is zero. This addresses the ~896 MB/min production allocation source. Commits and subsequent milestones - 54733537c `perf: Eliminate wall collision hot-loop allocations`: harness foundation + immutable wall index/tests. - 43114b0d6 `perf: Remove steady dynel collision allocations`: replaces the unsafe shared Dictionary<dynel,HashSet<char>> with character-partitioned occupancy. Each character has one gate/HashSet; parallel cells for unrelated characters do not contend, removal waits for an in-flight lease, invalidates the old generation and clears it atomically. Stable candidate checks use squared distance. Capturing async lambdas moved behind transition-only schedule helpers, so the steady path creates no display class. Existing delayed work remains off-thread and is explicitly not the final owner-tick scheduling design. - Dynel harness before: 80.007–80.018 B/candidate, 61,445,744 bytes and 5 minor GCs for 768k production-shape candidates. After: 5,448 aggregate harness bytes (0.007 B/candidate), no GC; smoke 936 bytes total (0.018 B/candidate). Kernel lease test is exactly zero after warmup. - Occupancy tests cover exact transitions, exact zero stable allocation, 1,000 parallel character partitions and teardown waiting/invalidation. Authored teleport/logging/static-door regressions pass. - 7cf803ba3 `perf: Replace quadratic fight tally reconciliation`: one concurrency-safe authoritative reverse attacker count receives unique hate-list and player FightingTarget add/remove transitions. Ten-second correction now compares O(1) authoritative state instead of scanning every NPC/player for every character. Existing next-tick correction broadcast semantics remain. Concurrent 10k increments/decrements are serialized per target and cannot go negative. - Fight harness before at 1,000 characters: 3m comparisons in 164ms, 384,160 bytes, max measured batch 128.9ms. After: same logical workload in ~2ms, 160 aggregate harness bytes, no GC, max ~0.67ms. Focused tests prove reconciliation does not access Npcs/Players and concurrent counts do not lose updates. - beeee1c18 `perf: Avoid corpse lifecycle snapshots`: hot owner tick now enumerates ConcurrentDictionary directly instead of allocating complete corpse/terminal arrays. This is intentionally weakly consistent but safe: removed entries may receive one idempotent tick exactly as with the old pre-taken snapshot; additions may process now or next frame. Ordinary List/Dictionary enumeration was not introduced. Rare despawn player snapshots remain unchanged. - Corpse production shape (2,000 stable corpses ×100 ticks): 15,808 constant harness bytes total (0.079 B/entry), no GC, versus production trace attribution of ~241 MB/min to snapshot-heavy corpse ticks. Remaining ~158 B/tick constant enumerator/harness overhead motivates the owner due-queue checklist; this slice removes proportional allocation but does not claim exact zero. Validation at this milestone - 44 focused wall/dynel/fight/hate tests pass; no skips. TheThreeEngines.sln builds with TreatWarningsAsErrors and no C# errors/warnings (eight known SDK package locale-normalization warnings). Harness/PFE build clean. git diff --check clean after each commit. - 0e81ea135 `perf: Spatially index static NPC dynels`: adds immutable 8-unit spatial buckets copied from static RDB dynels and atomically published per playfield. Parallel NPC workers query only the current/adjacent 3x3 buckets with lock-free reads. Character-owned occupancy maintains a warmed per-character identity buffer so already-entered dynels are still evaluated for departure even after a teleport moves outside nearby buckets; mutation/removal remains under the character partition gate. Static authored event references remain attached while spatial values are copied. - Dynamic terminals deliberately remain on safe ConcurrentDictionary enumeration until explicit terminal lifecycle publication is implemented; no stale copy is introduced. Player static-dynel special handling also retains its authored path for now. Thus this completes the dominant profiled NPC × static-dynel scan, not every dynel path. - Dynel production harness now represents 7.68m avoided static comparisons over 200 iterations/DOP8 in ~26ms with 16,496 aggregate harness bytes, no GC. At equivalent 20-iteration scale the indexed path is roughly 4x faster than the post-closure-fix full scan; pre-fix also allocated 80 B per comparison. Smoke stable path remains 936 total harness bytes. - Five index tests cover neighborhood selection, negative coordinates, copied values, exact zero warmed bucket reads and 100k concurrent readers. Combined focused suite is now 54 passes/no skips. TheThreeEngines build passes; only eight known SDK locale-normalization warnings. - 696713580 `fix: Run dynel effects on character ticks`: generic NPC/dynamic-terminal enter and exit effects no longer use arbitrary Task.Run/Task.Delay continuations that mutate Character state off-owner. Rare transitions enqueue TimedActionType.DynelCollision on that character; player/NPC heartbeat executes under its existing ownership lock. Entry actions revalidate playfield identity and occupancy generation/state before doing work; character removal invalidates pending entries. The static player-door special path still uses its legacy delayed path and remains explicit follow-up. - Two owner-scheduling tests prove standing inside queues exactly one 500ms character action and removal makes pending entry work stale. Combined dynel owner/index/occupancy suite: 23 passes/no skips. - f80f2847e `perf: Index dynamic dynels and own player effects`: completes the remaining dynel slice. - Adds immutable copy-on-write terminal spatial publication with lock-free readers. All engine terminal add/remove sites now use one lifecycle boundary; PfSystemManager dispatches topology events and DynelCollision republishes only on actual mutations/initial load. Removed entries are purged from all character occupancy under per-character gates. Authoritative dictionary lookup rejects a stale published source during concurrent removal. - Static player dynels now use the same nearby immutable index as NPCs while separately processing the character's small occupied set for long-teleport exits. The authored static door/ExitProxy/GM requirement behavior moved unchanged into a character-owned TimedActionType.DynelCollision callback. - Generic NPC/terminal and special player delayed effects now all run through character timed actions, rechecking playfield and occupancy before execution; arbitrary Task.Run/Task.Delay mutation paths are removed from DynelCollision. - Terminal index tests prove copied topology, zero-allocation warmed reads and 100k concurrent readers. Lifecycle tests prove add publication, removal publication/occupancy purge and no stale collision. Special player tests prove exactly one 500ms owner action. Broader dynel/terminal/encounter/pickup filter: 61 passed/no skips. Engine solution builds with zero errors/source warnings. - Final production-shaped static dynel run: 7.68m avoided comparisons, 16,304 aggregate harness bytes (0.002 B/logical avoided comparison), no GC, ~8ms elapsed in the sampled run. Timing is local evidence, not a CI threshold. - Dynel spatial/ownership checklist is complete. Task remains in progress for corpse/terminal due queues, harness concurrent topology/hate/teardown modes and aggregate/perf comparison. No production deployment. Pull request - #2574: perf: Reduce PFE heartbeat allocation and scans (merged into develop as 0028a53fe) Corpse/terminal scheduling follow-up started in /home/mathias/Projects/worktrees/engines-pfe-lifecycle on branch perf/schedule-pfe-lifecycle-deadlines from merged develop 0028a53fe. First map every mutation/open/loot/respawn path and preserve active-time/sleep semantics; no absolute wall-clock deadline or unsafe mutable enumeration substitution. - https://git.project-rk.com/prk/engines/pulls/2574 - Head perf/reduce-pfe-hot-loop-allocations -> develop. Seven focused commits pushed; worktree clean at PR creation. Ready for review; remaining task items are explicit follow-up rather than uncommitted PR work. Corpse/terminal deadline milestone - f25adfe7c `perf: Schedule corpse and terminal lifecycle deadlines` on perf/schedule-pfe-lifecycle-deadlines from merged develop. - Replaces full corpse/terminal tick traversal with an owner-only active-time PriorityQueue for corpse loot-open, natural expiry and opened-empty expiry; a separate wall-clock PriorityQueue preserves existing terminal `CreationTime + timeexist` semantics. No OnTick means corpse active lifetime pauses during playfield sleep, matching the prior delta decrement; temporary terminals still expire across sleep on the next tick. - Add/remove/change producers enqueue MPSC lifecycle commands. Owner drains into queues/sets. Per-identity generations prevent old corpse/terminal deadlines removing replacements with reused IDs. Cleanup invalidates generations and clears owner structures. - Only opened corpses are proximity-checked; only opened/looted corpse/chest candidates are watched until they become empty, then receive one 3-second deadline. Generic corpse/chest handlers publish state changes. Item removals need no extra hooks because opened candidates remain watched. Terminal lifecycle hooks from PR #2574 provide add/remove publication. - No mutable ordinary collection is concurrently enumerated: producer-facing maps/queue are concurrent; PriorityQueue/HashSets/scratch are heartbeat-owner-only. Concurrent 2,000 add/remove stress while OnTick drains passes. - Seven focused scheduler tests pass: active-time sleep semantics, opened-empty delay, corpse and terminal stale-generation rejection, loot-terminal transition, concurrent producer/owner drain and exact zero bytes over 1,000 warmed idle ticks. Broader corpse/terminal/pickup/encounter filter: 43 pass/no skips. TheThreeEngines builds with zero errors/source warnings. - Updated harness registers 2,000 real scheduler entries before measurement. 1,000 steady ticks represent 2m avoided scans in ~1ms, 10,208 aggregate harness/sample bytes (0.005 B/logical avoided entry), no GC; focused kernel idle assertion is exactly zero. - Corpse/terminal due-queue checklist complete. Remaining task scope: expand harness topology/hate/teardown modes and final aggregate/Linux perf comparison. No deployment. Pull request - #2575: perf: Schedule corpse and terminal lifecycle deadlines - https://git.project-rk.com/prk/engines/pulls/2575 - Head perf/schedule-pfe-lifecycle-deadlines -> develop. Commit f25adfe7c pushed; worktree clean. Ready for review.
Checklist
4 of 6 complete- Build the deterministic infrastructure-free hot-loop harness and record pre-fix serial/parallel production-shape baselines.
- Make wall collision allocation-free with immutable precomputed geometry/indexes and concurrency/behavior tests.
- Make steady dynel proximity allocation-free using immutable/versioned spatial indexes, character-owned occupancy and safe owner-tick delayed work.
- Replace quadratic FightTally reconciliation with concurrency-safe authoritative deltas and bounded versioned validation.
- Replace corpse/terminal per-tick snapshots with idempotent owner-drained lifecycle scheduling.
- Run behavior, allocation and concurrent lifecycle validation, then compare full harness and Linux perf profiles against baseline.
- Area
- Project Rubi-Ka
- Project
- engines
- Kind
- Bug
- Priority
- High
- Task ID
tsk_eqdu5zwbmf- Updated