From 119a2326beb2fecafa23faea4ca624dd52b85c63 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 10 Jul 2026 14:15:10 +0200 Subject: [PATCH] =?UTF-8?q?fix(#193):=20clear=20InstanceGroup.Opacities=20?= =?UTF-8?q?each=20frame=20=E2=80=94=20stop=20the=20~1=20GB/min=20LOH=20lea?= =?UTF-8?q?k?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (measured, not guessed): the OOM was a MANAGED Large-Object-Heap leak (~5 GB in 6 min of roaming; working set 1.6 GB -> 7.6 GB), dominated by ~3.8 GB of live System.Single[]. A heap-retention analysis of a captured gcdump traced 241 of 242 giant float arrays to a single List: WbDrawDispatcher.InstanceGroup.Opacities. #188 (2026-07-09, fading doors) added Opacities as a 5th per-instance parallel list alongside Matrices/Slots/LightSets/IndoorFlags, but left it out of the per-frame clear loop (WbDrawDispatcher.cs:959). So Opacities.Add(1.0f) fired once per drawn instance per frame and the list never reset — growing forever; List capacity doubling produced the observed ~128 MB / ~512 MB power-of-two arrays and OOM after ~50 min. Hit both UI builds because it's core render, and started only recently because Opacities did not exist before #188. Fix: extract the per-frame reset into InstanceGroup.ClearPerInstanceData() (promoted InstanceGroup private->internal) that clears ALL FIVE parallel lists in one place, so a future 6th list can't silently drift out of the frame lifecycle again. Line 959 now calls it. TDD: InstanceGroupClearTests asserts every parallel list resets (RED with the old 4-list clear, GREEN after adding Opacities.Clear()). Static-audit note: this is exactly why we measured before fixing — the audit ranked GlobalMeshBuffer (GPU) #1 and the entity-dict leak #3; both were wrong about the dominant cause. The dotnet-counters (managed vs native) + gcdump retention graph named the real one-line culprit. Secondary (NOT this commit): _groups dictionary is never pruned (bounded by distinct group keys, empty groups are cheap) — filed as a follow-up, not the multi-GB driver. Co-Authored-By: Claude Opus 4.8 --- .../Rendering/Wb/WbDrawDispatcher.cs | 23 ++++++++++-- .../Rendering/Wb/InstanceGroupClearTests.cs | 35 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 tests/AcDream.App.Tests/Rendering/Wb/InstanceGroupClearTests.cs diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 8fb2a90c..f1ede5bf 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -956,7 +956,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable camPos = invView.Translation; // ── Phase 1: clear groups, walk entities, build groups ────────────── - foreach (var grp in _groups.Values) { grp.Matrices.Clear(); grp.Slots.Clear(); grp.LightSets.Clear(); grp.IndoorFlags.Clear(); } + foreach (var grp in _groups.Values) grp.ClearPerInstanceData(); var metaTable = _meshAdapter.MetadataTable; uint anyVao = 0; @@ -2556,7 +2556,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable } } - private sealed class InstanceGroup + internal sealed class InstanceGroup { public uint Ibo; public uint FirstIndex; @@ -2598,5 +2598,24 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // layout time the dispatcher writes Opacities[i] into _alphaData at // the same cursor, so the binding=7 instanceAlpha[] tracks binding=0. public readonly List Opacities = new(); + + /// + /// Resets every per-instance parallel list for a new frame. These lists are + /// appended in lockstep (one entry per drawn instance) during group build, so + /// they MUST all be cleared together each frame. Keeping the reset in one + /// method stops a newly-added parallel list from silently drifting out of the + /// frame lifecycle — which is exactly the #193 OOM: #188 added + /// alongside the others but left it out of the old + /// inline clear loop, so it grew one float per instance per frame forever and + /// leaked ~1 GB/min of LOH float[] as its backing array doubled. + /// + public void ClearPerInstanceData() + { + Matrices.Clear(); + Slots.Clear(); + LightSets.Clear(); + IndoorFlags.Clear(); + Opacities.Clear(); + } } } diff --git a/tests/AcDream.App.Tests/Rendering/Wb/InstanceGroupClearTests.cs b/tests/AcDream.App.Tests/Rendering/Wb/InstanceGroupClearTests.cs new file mode 100644 index 00000000..967392e8 --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/Wb/InstanceGroupClearTests.cs @@ -0,0 +1,35 @@ +using System.Numerics; +using AcDream.App.Rendering.Wb; +using Xunit; + +namespace AcDream.App.Tests.Rendering.Wb; + +public class InstanceGroupClearTests +{ + // #193 (regression from #188, 2026-07-09): WbDrawDispatcher's InstanceGroup holds + // five per-instance parallel lists — Matrices, Slots, LightSets, IndoorFlags, + // Opacities — appended in lockstep (one entry per drawn instance) every frame. The + // per-frame reset must clear ALL of them. #188 added Opacities but left it out of + // the inline clear loop, so it grew one float per instance per frame forever; as + // List's backing array doubled it produced ~128 MB / ~512 MB LOH float[] + // arrays and leaked ~1 GB/min -> OOM after ~50 min of play. This test pins that + // every parallel list is reset, so a newly-added list can't silently drift again. + [Fact] + public void ClearPerInstanceData_ClearsEveryParallelPerInstanceList() + { + var grp = new WbDrawDispatcher.InstanceGroup(); + grp.Matrices.Add(Matrix4x4.Identity); + grp.Slots.Add(1u); + grp.LightSets.Add(-1); + grp.IndoorFlags.Add(0u); + grp.Opacities.Add(1.0f); + + grp.ClearPerInstanceData(); + + Assert.Empty(grp.Matrices); + Assert.Empty(grp.Slots); + Assert.Empty(grp.LightSets); + Assert.Empty(grp.IndoorFlags); + Assert.Empty(grp.Opacities); // #193 — the list that leaked + } +}