acdream/docs/superpowers/plans/2026-06-23-datlock-contention-fix.md

_datLock Contention Fix — Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Remove the _datLock the update thread waits on during a landblock apply, by having the streaming worker pre-read the dats the apply needs into a bundle that rides on LoadedLandblock.

Architecture: The worker already holds _datLock for its whole per-landblock build and already reads most of these dats. We package the parsed dat objects the apply consumes onto LoadedLandblock (which already carries dat objects). ApplyLoadedTerrainLocked then reads from the bundle instead of _dats.Get<T>, makes zero DatCollection calls, and its lock(_datLock) is deleted. No StreamingController/delegate change — the bundle flows on the existing LoadedLandblock.

Tech Stack: C# / .NET 10, DatReaderWriter.DBObjs, xUnit.

Design source: docs/superpowers/specs/2026-06-23-datlock-contention-fix-design.md (A1).

Safety basis (verified): _datLock's only cross-thread job is serializing DatCollection.Get<T>. Everything else the apply mutates is update-thread-only (_physicsEngine, ShadowObjects, _envCellRenderer, _cellVisibility, _worldState, _buildingRegistries) or already concurrent-safe (PhysicsDataCache is all ConcurrentDictionary). So once the apply makes no Get<T> call, the lock is unnecessary around it.

The six Get<T> sites in ApplyLoadedTerrainLocked to eliminate (grep _dats.Get< / _dats!.Get< within ApplyLoadedTerrainLocked; lines drift, match by symbol):

1. LandBlockInfo — id (LandblockId & 0xFFFF0000u) | 0xFFFEu (cell count + buildings).
2. EnvCell — per cell id firstCellId + offset.
3. Environment — id 0x0D000000u | envCell.EnvironmentId.
4. Setup — building shell, id building.ModelId (the _dats!.Get site in the CacheBuilding block).
5. GfxObj — per meshRef.GfxObjId in the BSP-cache loop.
6. Setup — entity-part setup in the ShadowObjects registration block.

Out of scope: the H3 entity-leak / GPU heat-up, the invisible-char-on-portal bug, and variant A2 (moving the CPU build off-thread). The [FRAME-DIAG] apparatus stays in the tree until verification, then is stripped in a final commit.

---

Task 1: Define PhysicsDatBundle and attach it to LoadedLandblock

Files:

• Create: src/AcDream.Core/World/PhysicsDatBundle.cs

• Modify: src/AcDream.Core/World/LoadedLandblock.cs

• Test: tests/AcDream.Core.Tests/World/PhysicsDatBundleTests.cs

• Step 1: Write the failing test

// tests/AcDream.Core.Tests/World/PhysicsDatBundleTests.cs
using System.Collections.Generic;
using AcDream.Core.World;
using DatReaderWriter.DBObjs;
using Xunit;

public class PhysicsDatBundleTests
{
    [Fact]
    public void Empty_ReturnsNullsAndEmptyMaps()
    {
        var b = PhysicsDatBundle.Empty;
        Assert.Null(b.Info);
        Assert.Empty(b.EnvCells);
        Assert.Empty(b.Environments);
        Assert.Empty(b.Setups);
        Assert.Empty(b.GfxObjs);
    }

    [Fact]
    public void LoadedLandblock_DefaultsBundleToNull_BackCompat()
    {
        var lb = new LoadedLandblock(0xA9B4FFFFu, new LandBlock(),
            System.Array.Empty<WorldEntity>());
        Assert.Null(lb.PhysicsDats);
    }
}

• Step 2: Run test to verify it fails

Run: dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter PhysicsDatBundleTests Expected: FAIL — PhysicsDatBundle / PhysicsDats do not exist (compile error).

• Step 3: Create the bundle type

// src/AcDream.Core/World/PhysicsDatBundle.cs
using System.Collections.Generic;
using DatReaderWriter.DBObjs;

namespace AcDream.Core.World;

/// <summary>
/// The parsed dat objects <c>ApplyLoadedTerrainLocked</c> needs, pre-read by the
/// streaming worker under <c>_datLock</c> so the apply makes ZERO DatCollection
/// calls and the update thread never blocks on the worker's lock. Keyed by the
/// same dat id the apply would have passed to <c>_dats.Get&lt;T&gt;</c>.
/// </summary>
public sealed record PhysicsDatBundle(
    LandBlockInfo? Info,
    IReadOnlyDictionary<uint, EnvCell> EnvCells,
    IReadOnlyDictionary<uint, Environment> Environments,
    IReadOnlyDictionary<uint, Setup> Setups,
    IReadOnlyDictionary<uint, GfxObj> GfxObjs)
{
    public static readonly PhysicsDatBundle Empty = new(
        null,
        new Dictionary<uint, EnvCell>(),
        new Dictionary<uint, Environment>(),
        new Dictionary<uint, Setup>(),
        new Dictionary<uint, GfxObj>());
}

• Step 4: Add the optional field to LoadedLandblock

// src/AcDream.Core/World/LoadedLandblock.cs
using DatReaderWriter.DBObjs;

namespace AcDream.Core.World;

public sealed record LoadedLandblock(
    uint LandblockId,
    LandBlock Heightmap,
    IReadOnlyList<WorldEntity> Entities,
    PhysicsDatBundle? PhysicsDats = null);

Trailing default keeps every existing 3-arg constructor call (tests, far-tier early-out) compiling unchanged.

• Step 5: Run test to verify it passes

Run: dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter PhysicsDatBundleTests Expected: PASS (both facts).

• Step 6: Commit

git add src/AcDream.Core/World/PhysicsDatBundle.cs src/AcDream.Core/World/LoadedLandblock.cs tests/AcDream.Core.Tests/World/PhysicsDatBundleTests.cs
git commit -m "feat(streaming): PhysicsDatBundle on LoadedLandblock (datLock fix scaffold)"

---

Task 2: Gather the bundle on the worker (BuildPhysicsDatBundle)

Files:

• Modify: src/AcDream.App/Rendering/GameWindow.cs (add BuildPhysicsDatBundle; call it in BuildLandblockForStreamingLocked)

The gather MIRRORS the apply's six read sites exactly, reading from _dats under the worker's existing _datLock. It only READS + fills maps — no build, no registration. Some reads (entity GfxObj/Setup) duplicate reads the worker already does for the mesh; that is fine (DatCollection caches; it is on the worker, off the contended path).

• Step 1: Add the gather method

Add a private method to GameWindow (near BuildLandblockForStreamingLocked). It is called from inside the worker build, which already holds _datLock.

// src/AcDream.App/Rendering/GameWindow.cs  (new private method)
// Pre-reads (under the worker's _datLock) every dat ApplyLoadedTerrainLocked
// consumes, so the apply can run lock-free. MIRRORS the apply's read sites.
private AcDream.Core.World.PhysicsDatBundle BuildPhysicsDatBundle(
    uint landblockId,
    System.Collections.Generic.IReadOnlyList<AcDream.Core.World.WorldEntity> entities)
{
    var envCells = new System.Collections.Generic.Dictionary<uint, DatReaderWriter.DBObjs.EnvCell>();
    var environments = new System.Collections.Generic.Dictionary<uint, DatReaderWriter.DBObjs.Environment>();
    var setups = new System.Collections.Generic.Dictionary<uint, DatReaderWriter.DBObjs.Setup>();
    var gfxObjs = new System.Collections.Generic.Dictionary<uint, DatReaderWriter.DBObjs.GfxObj>();

    // (1) LandBlockInfo
    var lbInfo = _dats!.Get<DatReaderWriter.DBObjs.LandBlockInfo>(
        (landblockId & 0xFFFF0000u) | 0xFFFEu);

    if (lbInfo is not null)
    {
        // (2)+(3) EnvCell + Environment, per cell
        if (lbInfo.NumCells > 0)
        {
            uint firstCellId = (landblockId & 0xFFFF0000u) | 0x0100u;
            for (uint offset = 0; offset < lbInfo.NumCells; offset++)
            {
                uint envCellId = firstCellId + offset;
                var envCell = _dats.Get<DatReaderWriter.DBObjs.EnvCell>(envCellId);
                if (envCell is null) continue;
                envCells[envCellId] = envCell;
                if (envCell.EnvironmentId == 0) continue;
                uint envId = 0x0D000000u | envCell.EnvironmentId;
                if (!environments.ContainsKey(envId))
                {
                    var environment = _dats.Get<DatReaderWriter.DBObjs.Environment>(envId);
                    if (environment is not null) environments[envId] = environment;
                }
            }
        }

        // (4) Building shell Setup, per building
        foreach (var building in lbInfo.Buildings)
        {
            uint modelId = building.ModelId;
            if ((modelId & 0xFF000000u) == 0x02000000u && !setups.ContainsKey(modelId))
            {
                var bldSetup = _dats.Get<DatReaderWriter.DBObjs.Setup>(modelId);
                if (bldSetup is not null) setups[modelId] = bldSetup;
            }
        }
    }

    // (5)+(6) Entity GfxObj + Setup (BSP cache + ShadowObjects parts)
    foreach (var entity in entities)
    {
        uint src = entity.SourceGfxObjOrSetupId;
        if ((src & 0xFF000000u) == 0x02000000u && !setups.ContainsKey(src))
        {
            var s = _dats.Get<DatReaderWriter.DBObjs.Setup>(src);
            if (s is not null) setups[src] = s;
        }
        foreach (var meshRef in entity.MeshRefs)
        {
            uint gid = meshRef.GfxObjId;
            if ((gid & 0xFF000000u) != 0x01000000u) continue;
            if (gfxObjs.ContainsKey(gid)) continue;
            var gfx = _dats.Get<DatReaderWriter.DBObjs.GfxObj>(gid);
            if (gfx is not null) gfxObjs[gid] = gfx;
        }
    }

    return new AcDream.Core.World.PhysicsDatBundle(lbInfo, envCells, environments, setups, gfxObjs);
}

NOTE: this mirrors the apply's iteration. If, while doing Task 4, you find the apply reads a dat id this gather does not produce, add it here in the same commit — gather and apply must enumerate the same ids.

• Step 2: Attach the bundle to the near-tier build result

In BuildLandblockForStreamingLocked, the near path returns a LoadedLandblock built from hydrated entities. Find the final return new ...LoadedLandblock(landblockId, heightmap, hydrated...) and add the bundle. The LoadFar early-out returns an empty-entity landblock — give it PhysicsDatBundle.Empty (far tier has no cells/buildings/entities; the apply's near-only side effects are skipped):

// LoadFar early-out return — add the empty bundle:
return new AcDream.Core.World.LoadedLandblock(
    landblockId,
    heightmapOnly,
    System.Array.Empty<AcDream.Core.World.WorldEntity>(),
    AcDream.Core.World.PhysicsDatBundle.Empty);

// Near-tier final return — add the gathered bundle (build it AFTER `hydrated`
// is populated so the entity GfxObj/Setup ids are known):
var physicsDats = BuildPhysicsDatBundle(landblockId, hydrated);
return new AcDream.Core.World.LoadedLandblock(
    landblockId,
    <existingHeightmapArg>,
    hydrated,
    physicsDats);

(Match <existingHeightmapArg> to whatever the current near return passes for Heightmap.)

• Step 3: Build to verify it compiles

Run: dotnet build src/AcDream.App/AcDream.App.csproj -c Debug --nologo Expected: Build succeeded, 0 errors.

• Step 4: Commit

git add src/AcDream.App/Rendering/GameWindow.cs
git commit -m "feat(streaming): worker pre-reads ApplyLoadedTerrain dats into the bundle"

---

Task 3: Consume the bundle in ApplyLoadedTerrainLocked (the six rewrites)

Files:

• Modify: src/AcDream.App/Rendering/GameWindow.cs (ApplyLoadedTerrainLocked)

The apply receives lb (the LoadedLandblock). Add one local at the top of ApplyLoadedTerrainLocked, then replace each _dats.Get<X>(id) with a bundle lookup. Do NOT remove the lock yet (Task 5) — this task only changes the data source so the diff is reviewable in isolation.

• Step 1: Add the bundle local at the top of ApplyLoadedTerrainLocked

Right after the existing if (_terrain is null || _dats is null || _heightTable is null) return; guard:

var datBundle = lb.PhysicsDats ?? AcDream.Core.World.PhysicsDatBundle.Empty;

• Step 2: Rewrite the six read sites

Replace each (match by surrounding context; lines drift). In every case the id expression is unchanged — only the source changes from _dats.Get<X>(id) to a bundle lookup. Use TryGetValue so a gather/apply id mismatch fails loudly rather than silently rendering wrong geometry:

// (1) LandBlockInfo  — was: _dats.Get<LandBlockInfo>((lb.LandblockId & 0xFFFF0000u) | 0xFFFEu)
var lbInfo = datBundle.Info;

// (2) EnvCell  — was: _dats.Get<EnvCell>(envCellId)
if (!datBundle.EnvCells.TryGetValue(envCellId, out var envCell)) continue;

// (3) Environment  — was: _dats.Get<Environment>(0x0D000000u | envCell.EnvironmentId)
if (!datBundle.Environments.TryGetValue(0x0D000000u | envCell.EnvironmentId, out var environment)) continue;

// (4) building Setup  — was: _dats!.Get<Setup>(building.ModelId)
datBundle.Setups.TryGetValue(building.ModelId, out var bldSetup);

// (5) GfxObj (BSP loop)  — was: _dats.Get<GfxObj>(meshRef.GfxObjId)
if (!datBundle.GfxObjs.TryGetValue(meshRef.GfxObjId, out var gfx)) continue;

// (6) entity-part Setup (ShadowObjects)  — was: _dats.Get<Setup>(src)
datBundle.Setups.TryGetValue(src, out var datSetup);

For (4) and (6) the originals already null-check the result, so a TryGetValue that leaves the out-var null preserves identical behavior. For (2),(3),(5) the originals continue on null — TryGetValue false maps to the same continue.

After this task there must be zero _dats. references left inside ApplyLoadedTerrainLocked. Verify: grep -n "_dats" GameWindow.cs shows no hit between the method's open brace and its close brace.

• Step 3: Build

Run: dotnet build src/AcDream.App/AcDream.App.csproj -c Debug --nologo Expected: Build succeeded, 0 errors.

• Step 4: Run the physics/collision conformance suite (behavior regression)

Run: dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj Expected: PASS — the physics surfaces / BSP / cell tests exercise the same data; identical ids → identical cached dat objects → identical results.

• Step 5: Commit

git add src/AcDream.App/Rendering/GameWindow.cs
git commit -m "refactor(apply): read dats from the bundle, not DatCollection (no lock change yet)"

---

Task 4: Remove the apply's lock(_datLock)

Files:

• Modify: src/AcDream.App/Rendering/GameWindow.cs (ApplyLoadedTerrain)

• Step 1: Drop the lock wrapper

In ApplyLoadedTerrain (the wrapper, NOT …Locked), replace:

lock (_datLock)
{
    ApplyLoadedTerrainLocked(lb, meshData);
}

with:

// The apply makes zero DatCollection calls (Task 3) — all dats come from
// lb.PhysicsDats, pre-read on the worker. Its remaining mutations are
// update-thread-only or ConcurrentDictionary-safe (PhysicsDataCache), so the
// dat lock is no longer needed here. Removing it eliminates the measured
// 24ms-median / 88ms-p95 lockwait stall (the FPS 30↔200 swing).
ApplyLoadedTerrainLocked(lb, meshData);

(Keep the [FRAME-DIAG] lockwait timing wrapper around this call if present — it is how we prove the wait drops to ~0. Leave the [FRAME-DIAG] apparatus in place until Task 5 verification.)

• Step 2: Build + full test suite

Run: dotnet build -c Release --nologo then dotnet test Expected: Build succeeded; all tests PASS.

• Step 3: Commit

git add src/AcDream.App/Rendering/GameWindow.cs
git commit -m "fix(streaming): drop _datLock from the terrain apply (FPS swing root cause)"

---

Task 5: Empirical verification + apparatus strip

Files:

• Modify: src/AcDream.App/Rendering/GameWindow.cs, src/AcDream.App/Streaming/StreamingController.cs (strip [FRAME-DIAG])

• Modify: docs/superpowers/specs/2026-06-23-datlock-contention-fix-design.md (mark verified; correct §5 to note PhysicsDataCache is ConcurrentDictionary)

• Step 1: Build Release

Run: dotnet build src/AcDream.App/AcDream.App.csproj -c Release --nologo Expected: Build succeeded.

• Step 2: User runs the verification capture

User launches Release with ACDREAM_WB_DIAG=1, repeats the repro (town walk + several portal-hops as notan → +Je), closes the window. Acceptance: in the [FRAME-DIAG] lines, lockwait median+p95 drop from ~24/88 ms to ~0; apply collapses toward its in-lock body cost; and the visible 30↔200 swing is gone while streaming/portal-hopping. (This is a STOP-for-user step — do not claim success without the user's capture.)

• Step 3: Strip the [FRAME-DIAG] apparatus

Remove every [FRAME-DIAG] field, the _frameDiag gate, the FrameDiagPush helper, the apply/upload/cell/bsp/shadow/lockwait accumulators + sample arrays, the OnUpdate flush block, the in-body checkpoints, the MaybeFlushTerrainDiag [FRAME-DIAG] line, and the StreamingController DeferredApplyBacklog / ForceReloadCount / LastForceReloadDropCount probe surface. Leave the pre-existing [TERRAIN-DIAG] rollup intact.

• Step 4: Update the design doc

Mark the spec verified with the before/after lockwait numbers, and correct §5 to state PhysicsDataCache is ConcurrentDictionary (so its shared writes were never _datLock-dependent).

• Step 5: Build + test green, commit

Run: dotnet build -c Release --nologo then dotnet test Expected: green.

git add -A
git commit -m "chore: strip [FRAME-DIAG] probes; datLock FPS fix verified"

• Step 6: Update ISSUES / roadmap

Move the FPS-swing item to Recently-closed in docs/ISSUES.md with the fix SHA; note H3 (entity-leak/GPU heat-up) and the invisible-char bug remain OPEN.

---

Self-review

• Spec coverage: worker pre-read (Task 2) ✓; bundle on LoadedLandblock (Task 1) ✓; six Get sites rewritten (Task 3) ✓; lock removed (Task 4) ✓; empirical lockwait→0 (Task 5) ✓; apparatus strip (Task 5) ✓; H3/invisible-char/A2 excluded ✓.
• Test note (deviation from spec wording): the spec named a "golden equivalence test." A GL-bound apply is not unit-testable in isolation, so equivalence is instead guaranteed-by-construction (identical ids → identical cached dat objects → identical math), enforced at runtime by the TryGetValue loud-fail in Task 3 Step 2, regression-covered by the existing physics/collision conformance suite (Task 3 Step 4), and proven empirically by Task 5. This satisfies the spec's intent more practically than a synthetic golden test.
• Placeholder scan: <existingHeightmapArg> in Task 2 Step 2 is an intentional match-to-source marker, not a placeholder — the engineer copies the current near-return heightmap argument.
• Type consistency: PhysicsDatBundle fields (Info, EnvCells, Environments, Setups, GfxObjs) are used identically in Task 1 (definition), Task 2 (population), Task 3 (consumption). PhysicsDats property name consistent across Tasks 1–3.