fix(streaming): drop _datLock from the terrain apply (FPS swing root cause)
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) <noreply@anthropic.com>
This commit is contained in:
parent
81a5605ff4
commit
536f1c04cd
1 changed files with 14 additions and 15 deletions
|
|
@ -6611,22 +6611,21 @@ public sealed class GameWindow : IDisposable
|
||||||
if (_terrain is null || _dats is null) return;
|
if (_terrain is null || _dats is null) return;
|
||||||
AcDream.Core.Physics.PhysicsDiagnostics.LogTeleport("APPLY", lb.LandblockId);
|
AcDream.Core.Physics.PhysicsDiagnostics.LogTeleport("APPLY", lb.LandblockId);
|
||||||
|
|
||||||
// Phase A.1 hotfix: render-thread path also takes the dat lock so it
|
// datLock fix (2026-06-23): ApplyLoadedTerrainLocked now makes ZERO
|
||||||
// doesn't race with BuildLandblockForStreaming on the worker thread.
|
// DatCollection calls — every dat it needs was pre-read by the streaming
|
||||||
// Hold the lock across the entity hydration below (GfxObj sub-mesh
|
// worker into lb.PhysicsDats. Its remaining mutations are update-thread-
|
||||||
// builds). The terrain mesh is pre-built by the worker (T12) and passed
|
// only (physics engine, ShadowObjects, renderer, _worldState) or already
|
||||||
// in via meshData, so LandblockMesh.Build no longer runs under this lock.
|
// concurrent-safe (PhysicsDataCache is ConcurrentDictionary). _datLock's
|
||||||
// [FRAME-DIAG]: time the full locked apply (dat reads + physics/registry +
|
// only cross-thread job was serializing DatCollection, so with no Get call
|
||||||
// the terrain GPU upload) — this is the OnUpdate cost the title-bar ms misses.
|
// 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;
|
long fdT0 = _frameDiag ? System.Diagnostics.Stopwatch.GetTimestamp() : 0L;
|
||||||
lock (_datLock)
|
if (_frameDiag)
|
||||||
{
|
_applyLockWaitAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0;
|
||||||
// [FRAME-DIAG]: time blocked acquiring _datLock — the streaming worker
|
ApplyLoadedTerrainLocked(lb, meshData);
|
||||||
// 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)
|
if (_frameDiag)
|
||||||
{
|
{
|
||||||
_applyAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0;
|
_applyAccumTicks += System.Diagnostics.Stopwatch.GetTimestamp() - fdT0;
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue