From f918b3ea2cf90b6c2e4936e2a18b168c3dd78285 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 22 Jun 2026 13:03:20 +0200 Subject: [PATCH] refactor(teleport A): O(N) deferred drain + never-arrives test (review fixes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace RemoveAt(0)-in-a-loop drain idiom with RemoveRange(0, i) in both Step-1 deferred drain and the post-found deferred drain inside DrainAndApply, making each an O(N) single shift instead of O(N²) on the render-thread hot path. Add PriorityNeverArrives_noThrow_noLoss_noDoubleApply test: sets a priority id that never appears in the outbox, ticks several times, asserts no throw, no loss of the non-priority completions, and no double-apply (applied count == completions enqueued). Comment above the priority-hunt explains the failure mode: completions relocate to _deferredApply while the hunt is active and drain at per-frame budget until the caller clears PriorityLandblockId. Restyle test 2 to use named constructor arguments and break the compressed lambda onto readable lines (matches test 1 style). Co-Authored-By: Claude Sonnet 4.6 --- .../Streaming/StreamingController.cs | 23 ++---- .../StreamingControllerPriorityApplyTests.cs | 72 +++++++++++++++++-- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/AcDream.App/Streaming/StreamingController.cs b/src/AcDream.App/Streaming/StreamingController.cs index 30b690cc..f5f9f43d 100644 --- a/src/AcDream.App/Streaming/StreamingController.cs +++ b/src/AcDream.App/Streaming/StreamingController.cs @@ -336,15 +336,10 @@ public sealed class StreamingController // --- Step 1: drain the deferred buffer first (items held from a prior // priority-hunt that overshot the cap). Apply up to `budget` of them. - int deferredApplied = 0; - while (deferredApplied < budget && _deferredApply.Count > 0) - { - var item = _deferredApply[0]; - _deferredApply.RemoveAt(0); - ApplyResult(item); - deferredApplied++; - } - budget -= deferredApplied; + int i1 = 0; + while (i1 < budget && i1 < _deferredApply.Count) { ApplyResult(_deferredApply[i1]); i1++; } + if (i1 > 0) _deferredApply.RemoveRange(0, i1); + budget -= i1; if (budget <= 0) return; @@ -385,13 +380,9 @@ public sealed class StreamingController { // Now apply up to remaining budget from deferred (mix from the // chunk we just buffered + any pre-existing deferred items). - while (budget > 0 && _deferredApply.Count > 0) - { - var item = _deferredApply[0]; - _deferredApply.RemoveAt(0); - ApplyResult(item); - budget--; - } + int i2 = 0; + while (i2 < budget && i2 < _deferredApply.Count) { ApplyResult(_deferredApply[i2]); i2++; } + if (i2 > 0) _deferredApply.RemoveRange(0, i2); return; } // Priority not found in the outbox this frame: fall through to diff --git a/tests/AcDream.Core.Tests/Streaming/StreamingControllerPriorityApplyTests.cs b/tests/AcDream.Core.Tests/Streaming/StreamingControllerPriorityApplyTests.cs index d3af4b57..4668b867 100644 --- a/tests/AcDream.Core.Tests/Streaming/StreamingControllerPriorityApplyTests.cs +++ b/tests/AcDream.Core.Tests/Streaming/StreamingControllerPriorityApplyTests.cs @@ -68,10 +68,19 @@ public class StreamingControllerPriorityApplyTests var applied = new List(); var state = new GpuWorldState(); var ctrl = new StreamingController( - (_, _) => { }, _ => { }, - max => { var b = new List(); while (b.Count < max && outbox.Count > 0) b.Add(outbox.Dequeue()); return b; }, - (lb, _) => applied.Add(lb.LandblockId), - state, 4, 12) { MaxCompletionsPerFrame = 4 }; + enqueueLoad: (_, _) => { }, + enqueueUnload: _ => { }, + drainCompletions: max => + { + var batch = new List(); + while (batch.Count < max && outbox.Count > 0) batch.Add(outbox.Dequeue()); + return batch; + }, + applyTerrain: (lb, _) => applied.Add(lb.LandblockId), + state: state, + nearRadius: 4, + farRadius: 12) + { MaxCompletionsPerFrame = 4 }; ctrl.PriorityLandblockId = priority; ctrl.Tick(169, 180); // priority + some others @@ -82,4 +91,59 @@ public class StreamingControllerPriorityApplyTests Assert.Contains(priority, applied); Assert.Equal(6, applied.Count); // all six applied, none lost } + + [Fact] + public void PriorityNeverArrives_noThrow_noLoss_noDoubleApply() + { + // While a PriorityLandblockId is set but the matching completion never + // arrives (e.g. the load failed), the hunt moves outbox completions into + // _deferredApply and they drain at the per-frame budget — same backpressure + // as the normal throttle, just relocated — until the caller clears + // PriorityLandblockId (the TAS MaxContinue safety net does this on timeout). + uint priority = StreamingRegion.EncodeLandblockIdForTest(169, 180); + uint otherId0 = StreamingRegion.EncodeLandblockIdForTest(1, 0); + uint otherId1 = StreamingRegion.EncodeLandblockIdForTest(1, 1); + uint otherId2 = StreamingRegion.EncodeLandblockIdForTest(1, 2); + + var outbox = new Queue(new LandblockStreamResult[] + { + LoadedOf(otherId0), + LoadedOf(otherId1), + LoadedOf(otherId2), + }); + + var applied = new List(); + var state = new GpuWorldState(); + var ctrl = new StreamingController( + enqueueLoad: (_, _) => { }, + enqueueUnload: _ => { }, + drainCompletions: max => + { + var batch = new List(); + while (batch.Count < max && outbox.Count > 0) batch.Add(outbox.Dequeue()); + return batch; + }, + applyTerrain: (lb, _) => applied.Add(lb.LandblockId), + state: state, + nearRadius: 4, + farRadius: 12) + { MaxCompletionsPerFrame = 4 }; + + // Set a priority id that will NEVER appear in the outbox. + ctrl.PriorityLandblockId = priority; + + // Tick several times — should not throw and every non-priority completion + // must still be applied without duplication. + ctrl.Tick(169, 180); + ctrl.Tick(169, 180); + ctrl.Tick(169, 180); + + // (a) no throw — covered by reaching here without exception + // (b) every non-priority completion is applied (no loss) + Assert.Contains(otherId0, applied); + Assert.Contains(otherId1, applied); + Assert.Contains(otherId2, applied); + // (c) no double-apply: applied count matches completions enqueued + Assert.Equal(3, applied.Count); + } }