From 536f1c04cd459305ab0c101a92ff9033a03ae346 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 23 Jun 2026 09:37:50 +0200 Subject: [PATCH] fix(streaming): drop _datLock from the terrain apply (FPS swing root cause) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ApplyLoadedTerrainLocked makes zero DatCollection calls (all dats pre-read by the worker into lb.PhysicsDats), and its other mutations are update-thread-only or ConcurrentDictionary-safe, so the dat lock is unnecessary around it. Removing it eliminates the measured 24ms-median / 88ms-p95 lockwait stall that was the 30↔200 FPS swing. The worker still serializes its own dat reads on _datLock; only the apply stops contending. Build + 1568 Core tests green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 29 ++++++++++++------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index d9b7ed0d..78141c61 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -6611,22 +6611,21 @@ public sealed class GameWindow : IDisposable if (_terrain is null || _dats is null) return; AcDream.Core.Physics.PhysicsDiagnostics.LogTeleport("APPLY", lb.LandblockId); - // Phase A.1 hotfix: render-thread path also takes the dat lock so it - // doesn't race with BuildLandblockForStreaming on the worker thread. - // Hold the lock across the entity hydration below (GfxObj sub-mesh - // builds). The terrain mesh is pre-built by the worker (T12) and passed - // in via meshData, so LandblockMesh.Build no longer runs under this lock. - // [FRAME-DIAG]: time the full locked apply (dat reads + physics/registry + - // the terrain GPU upload) — this is the OnUpdate cost the title-bar ms misses. + // datLock fix (2026-06-23): ApplyLoadedTerrainLocked now makes ZERO + // DatCollection calls — every dat it needs was pre-read by the streaming + // worker into lb.PhysicsDats. Its remaining mutations are update-thread- + // only (physics engine, ShadowObjects, renderer, _worldState) or already + // concurrent-safe (PhysicsDataCache is ConcurrentDictionary). _datLock's + // only cross-thread job was serializing DatCollection, so with no Get call + // here the lock is unnecessary — removing it eliminates the measured + // 24ms-median / 88ms-p95 lockwait stall (the FPS 30↔200 swing). The method + // keeps its historical "Locked" name; the worker still serializes its OWN + // dat reads on _datLock — only the apply stops contending for it. + // [FRAME-DIAG]: lockwait now measures ~0 — the before/after proof. long fdT0 = _frameDiag ? System.Diagnostics.Stopwatch.GetTimestamp() : 0L; - lock (_datLock) - { - // [FRAME-DIAG]: time blocked acquiring _datLock — the streaming worker - // holds it for the full per-landblock build (tens of ms), stalling us. - if (_frameDiag) - _applyLockWaitAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0; - ApplyLoadedTerrainLocked(lb, meshData); - } + if (_frameDiag) + _applyLockWaitAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0; + ApplyLoadedTerrainLocked(lb, meshData); if (_frameDiag) { _applyAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0;