feat(pathfinding): non-eager TryGetMask + second-touch promotion (#2451)
## Summary Closes the Cold-cache regression flagged in PR #2450. `StepCache.TryGetMask` no longer eagerly runs `BuildChunk` on the first miss for a chunk that isn't in a `.swb` lazy reader. Instead it returns `Fallthrough_NotBuilt` and the caller (`BitmapAStarAlgorithm`) takes the per-cell slow path. The chunk is only promoted to the bitmap fast path after the **second** miss within a 30-second window, filtering single-touch pass-throughs. This makes BitmapAStar's worst-case (cold cache + short hops) collapse from **12–47× slower** than FastAStar to **roughly the same**, which is the floor the slow path can deliver. Steady-state warm performance (the actual deliverable) is unchanged from PR-5 — it was always the cache fast path. ## The pet-follow scenario this fixes A mounted player at ~4 tiles/sec with a pet/hireable following will trigger an NPC pathfind every 100–300 ms. Each pathfind is 1–6 tiles. As the player crosses chunk boundaries (~4 sec/chunk), the pet's first pathfind in the new chunk under the previous behavior triggered a full ~700 µs `BuildChunk` for a chunk the player would leave shortly after. At 50–100 mobiles per shard, this exceeded the 8 ms tick budget. PR-5 BDN data showed scenarios 6–9 (2–8 tile NPC perception) at 2,300–3,700 µs Cold vs FastAStar's 80–200 µs. Under the new gate: - First miss → `Fallthrough_NotBuilt` → caller uses slow path (~30–50 µs short path). No `BuildChunk`. No allocation. - Player keeps moving → chunk never gets a second touch within window → never promoted, no rot. - NPC patrolling a fixed territory → repeatedly hits the same chunks → second touch within window → promote → cache fast path on subsequent calls. ## What changed - **`CacheHitKind.Fallthrough_NotBuilt = 6`** + **`CacheStats.FallthroughNotBuilt`** counter. `IsHit=false`, so the caller routes to slow path. - **`StepCache._chunkMissTracker`** — `Dictionary<long, ChunkMissState>` capped at 4096 entries. State is `(byte missCount, uint lastMissTickStamp)` keyed by chunk key. Window-expired entries reset count to 1; capacity overflow prunes window-old entries first. - **`StepCache.MissPromotionThreshold`** (default `2`) and **`StepCache.MissPromotionWindowMs`** (default `30_000`) — tunable, can be wired through `ServerConfiguration` if shards want different policy. Setting threshold to `1` restores legacy eager-build behavior (used by tests that prime chunks via single `TryGetMask` call). - **`StepCache.TryGetMask` miss branch** — try lazy reader first (file-loaded chunks bypass the tracker entirely; an `.swb` represents an explicit prior decision to keep the chunk warm). Otherwise consult the tracker. - **`BitmapAStarAlgorithm.GetSuccessorsSlowPath`** now layers `IsBlockedByDynamic` on top of `CalcMoves.CheckMovement`. Previously the slow path only ran for `CanFly` creatures and rare cache fallthroughs — `CheckMovement` doesn't iterate same-cell mobiles, so the bitmap fast path's `IsBlockedByDynamic` was the only mobile-blocking check. Now first-touch pathfinds run through the slow path, so the gap had to close. ## Tests 50 pathfinding tests pass (was 47). New / updated: - **`TryGetMask_FirstTouchOnUnbuiltChunk_DefersBuildAndReturnsFallthrough`** — single TryGetMask call returns `Fallthrough_NotBuilt`, no chunk built, no allocation. - **`TryGetMask_SecondTouchWithinWindow_PromotesAndBuilds`** — second call inside the 30s window builds + serves. - **`TryGetMask_SecondTouchAfterWindow_RestartsCounterAndDefers`** — second call outside the window restarts the count, returns Fallthrough again. - **`TryGetMask_DistinctChunks_TrackedIndependently`** — counters are per-chunk; one touch on each of two adjacent chunks both stay in fallthrough. - **`LazyReaderHit_BypassesMissTrackerOnFirstTouch`** — open `.swb` + first touch hits without consulting the tracker. Production with `.swb` loaded skips the gate entirely. - **`MultisVersion_Bump_TriggersDirtyRebuild`** — updated to reflect the new 3-step flow (Fallthrough → Miss_NotBuilt → Miss_DirtyRebuild). - Tests that prime chunks via a single `TryGetMask` call (multi-Z, Tier4, lifecycle, parity, BitmapAStar uses-cache) set `MissPromotionThreshold = 1` to opt into eager behavior. ## Expected BDN impact The Cold column from PR-5's BDN should change as follows once the bench's submodule pointer is updated to this branch: | # | Scenario | Cold (PR-5) | Cold (PR-6 expected) | FastAStar Cold | |--:|-----------------|-------------:|---------------------:|---------------:| | 2 | sewer corridor | 1,627 µs | ~36 µs | 36 µs | | 4 | causeway | 1,533 µs | ~39 µs | 39 µs | | 6 | pet 2-tile | 2,364 µs | ~80 µs | 81 µs | | 8 | npc 5-tile | 3,708 µs | ~140 µs | 141 µs | | 9 | npc 8-tile | 2,386 µs | ~200 µs | 197 µs | WarmNoFile and LazyWarm rows should be unchanged — they were always cache-warm. The miss tracker only fires when neither resident chunks nor the lazy reader can satisfy the request. ## Future work (not in this PR) - **Background-thread bake**: builds outside the game thread so even promoted chunks don't pay the 700 µs build cost on the main thread. Rule 10 (no Task.Run) applies, so this needs careful design — the bake is a pure data transform but main-thread synchronization on chunk-state transitions has to be threaded through. Defer to a follow-up. - **Long-traverse BDN scenario**: a multi-Find benchmark simulating 50 pet repaths across chunk transitions. Requires restructuring the bench harness; the existing 10-scenario corpus + Cold provider already exercises the gate. - **Swim sourceZ bake**: scenario 5 (sea serpent) shows 56 B alloc on warm paths because the cache's SourceZ is computed under default-walker rules. Swim creatures fall through to slow path. Independent of this PR.
This commit is contained in:
Kamron Batman
GitHub
parent
cff9fbda29
commit
9a3d88988c
14 changed files with 1664 additions and 163 deletions
|
|
@ -1,3 +1,4 @@
|
|||
using System;
|
||||
using System.IO;
|
||||
using Server.Engines.Pathing.Cache;
|
||||
using Xunit;
|
||||
|
|
@ -17,15 +18,30 @@ public class StepCacheFileTests
|
|||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1; // eager build to populate chunks for save
|
||||
|
||||
var map = Map.Maps[1];
|
||||
Assert.NotNull(map);
|
||||
|
||||
// Populate three distinct chunks by querying different sectors.
|
||||
// Populate three distinct chunks by querying different sectors. Query at each cell's
|
||||
// real standable surface Z (where the cache anchors) so first-touch yields a clean
|
||||
// hit rather than an off-surface fallthrough.
|
||||
var sourceQueries = new[] { (1500, 1600), (1516, 1600), (1500, 1616) };
|
||||
foreach (var (x, y) in sourceQueries)
|
||||
var standZ = new sbyte[sourceQueries.Length];
|
||||
{
|
||||
cache.TryGetMask(map, x, y, sourceZ: 10);
|
||||
Span<sbyte> surfZ = stackalloc sbyte[16];
|
||||
for (var i = 0; i < sourceQueries.Length; i++)
|
||||
{
|
||||
var (qx, qy) = sourceQueries[i];
|
||||
var n = StepProbe.ComputeStandableSurfaceZs(map, qx, qy, surfZ);
|
||||
Assert.True(n > 0, $"({qx},{qy}) has no standable surface — bad test cell");
|
||||
standZ[i] = surfZ[0];
|
||||
}
|
||||
}
|
||||
for (var i = 0; i < sourceQueries.Length; i++)
|
||||
{
|
||||
var (x, y) = sourceQueries[i];
|
||||
cache.TryGetMask(map, x, y, standZ[i]);
|
||||
}
|
||||
|
||||
Assert.Equal(3, cache.GetStats().ResidentChunks);
|
||||
|
|
@ -36,7 +52,7 @@ public class StepCacheFileTests
|
|||
for (var i = 0; i < sourceQueries.Length; i++)
|
||||
{
|
||||
var (x, y) = sourceQueries[i];
|
||||
expected[i] = cache.TryGetMask(map, x, y, sourceZ: 10);
|
||||
expected[i] = cache.TryGetMask(map, x, y, standZ[i]);
|
||||
}
|
||||
|
||||
var path = Path.Combine(Path.GetTempPath(), $"step-cache-roundtrip-{System.Guid.NewGuid():N}.swb");
|
||||
|
|
@ -66,7 +82,7 @@ public class StepCacheFileTests
|
|||
for (var i = 0; i < sourceQueries.Length; i++)
|
||||
{
|
||||
var (x, y) = sourceQueries[i];
|
||||
var lookup = cache.TryGetMask(map, x, y, sourceZ: 10);
|
||||
var lookup = cache.TryGetMask(map, x, y, standZ[i]);
|
||||
Assert.Equal(CacheHitKind.Miss_NotBuilt, lookup.HitKind);
|
||||
Assert.Equal(expected[i].WalkMask, lookup.WalkMask);
|
||||
Assert.Equal(expected[i].WetMask, lookup.WetMask);
|
||||
|
|
@ -171,6 +187,7 @@ public class StepCacheFileTests
|
|||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1; // eager build to populate chunks for save
|
||||
|
||||
var map = Map.Maps[1];
|
||||
Assert.NotNull(map);
|
||||
|
|
@ -210,4 +227,175 @@ public class StepCacheFileTests
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// First-touch on a chunk that the lazy reader can satisfy must NOT route through the
|
||||
/// miss tracker — file-loaded chunks represent an explicit prior decision to keep
|
||||
/// them warm. This guards the deployment shape where an admin ships .swb files and
|
||||
/// expects the very first NPC pathfind in any region to use cache (not slow path).
|
||||
/// </summary>
|
||||
/// <summary>
|
||||
/// A chunk with an injected swim layer must serialize and deserialize via the lazy
|
||||
/// reader without losing the layer. Validates v3 file format end-to-end: swim layer
|
||||
/// fields survive Save → Clear → LazyOpen → first-touch query.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void SwimLayer_RoundTrips_ThroughLazyReader()
|
||||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1;
|
||||
|
||||
var map = Map.Maps[1];
|
||||
Assert.NotNull(map);
|
||||
|
||||
// Build a chunk and inject a synthetic swim layer onto cell (1500, 1600).
|
||||
cache.TryGetMask(map, 1500, 1600, sourceZ: 10);
|
||||
|
||||
var chunksField = typeof(StepCache).GetField(
|
||||
"_chunks",
|
||||
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance
|
||||
);
|
||||
var chunks = (System.Collections.Generic.Dictionary<long, StepChunk>)chunksField!.GetValue(cache)!;
|
||||
var key = StepCache.EncodeKey(map.MapID, 1500 >> 4, 1600 >> 4);
|
||||
var chunk = chunks[key];
|
||||
|
||||
chunk.AllocateSwimLayer();
|
||||
var cellIndex = ((1600 - ((1600 >> 4) << 4)) << 4) | (1500 - ((1500 >> 4) << 4));
|
||||
chunk.SwimSourceZ[cellIndex] = -7;
|
||||
chunk.SwimMask[cellIndex] = 0b0000_1111;
|
||||
chunk.SwimZN_Layer[cellIndex] = -7;
|
||||
chunk.SwimZNE_Layer[cellIndex] = -7;
|
||||
chunk.SwimZE_Layer[cellIndex] = -7;
|
||||
chunk.SwimZSE_Layer[cellIndex] = -7;
|
||||
|
||||
var path = Path.Combine(Path.GetTempPath(), $"step-cache-swim-{System.Guid.NewGuid():N}.swb");
|
||||
try
|
||||
{
|
||||
Assert.Equal(1, cache.SaveToFile(path, map.MapID));
|
||||
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1;
|
||||
Assert.True(cache.TryOpenLazyReader(path, map.MapID));
|
||||
|
||||
// Pull the chunk back via a query at swim Z; the layer must hit and serve our
|
||||
// injected mask. Walk-Z query of the same cell should still hit the walk
|
||||
// layer with whatever the bake produced.
|
||||
var swim = cache.TryGetMask(map, 1500, 1600, sourceZ: -7);
|
||||
Assert.True(swim.IsHit);
|
||||
Assert.Equal((byte)0, swim.WalkMask);
|
||||
Assert.Equal((byte)0b0000_1111, swim.WetMask);
|
||||
Assert.Equal((sbyte)-7, swim.SwimZ_N);
|
||||
Assert.Equal((sbyte)-7, swim.SwimZ_E);
|
||||
}
|
||||
finally
|
||||
{
|
||||
cache.Clear();
|
||||
if (File.Exists(path))
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// PreloadOnLazyOpen=true must materialize every chunk in the .swb file into the
|
||||
/// resident set immediately, eliminating first-touch file-read latency. Counterpart
|
||||
/// to <see cref="LazyReader_DoesNotMaterializeUntilQueried"/> which proves the
|
||||
/// default lazy behavior.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void TryOpenLazyReader_WithPreloadFlag_MaterializesAllChunksImmediately()
|
||||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1;
|
||||
|
||||
var map = Map.Maps[1];
|
||||
Assert.NotNull(map);
|
||||
|
||||
var coords = new (int, int)[]
|
||||
{
|
||||
(1500, 1600), (1516, 1600), (1500, 1616), (1516, 1616), (1532, 1600)
|
||||
};
|
||||
foreach (var (x, y) in coords)
|
||||
{
|
||||
cache.TryGetMask(map, x, y, sourceZ: 10);
|
||||
}
|
||||
|
||||
var path = Path.Combine(Path.GetTempPath(), $"step-cache-preload-{System.Guid.NewGuid():N}.swb");
|
||||
try
|
||||
{
|
||||
Assert.Equal(coords.Length, cache.SaveToFile(path, map.MapID));
|
||||
|
||||
cache.Clear();
|
||||
cache.PreloadOnLazyOpen = true;
|
||||
try
|
||||
{
|
||||
Assert.True(cache.TryOpenLazyReader(path, map.MapID));
|
||||
|
||||
// Every chunk should be resident — no further queries needed.
|
||||
Assert.Equal(coords.Length, cache.GetStats().ResidentChunks);
|
||||
Assert.Equal(0L, cache.GetStats().BuildsTotal); // came from file, not baker
|
||||
|
||||
// Subsequent query is a clean Hit, not a Miss_NotBuilt.
|
||||
var lookup = cache.TryGetMask(map, coords[0].Item1, coords[0].Item2, sourceZ: 10);
|
||||
Assert.Equal(CacheHitKind.Hit, lookup.HitKind);
|
||||
Assert.Equal(coords.Length, cache.GetStats().ResidentChunks);
|
||||
}
|
||||
finally
|
||||
{
|
||||
cache.PreloadOnLazyOpen = false;
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
cache.Clear();
|
||||
if (File.Exists(path))
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LazyReaderHit_BypassesMissTrackerOnFirstTouch()
|
||||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 1; // eager build for save phase
|
||||
|
||||
var map = Map.Maps[1];
|
||||
|
||||
// Build + save one chunk.
|
||||
cache.TryGetMask(map, 1500, 1600, sourceZ: 10);
|
||||
var path = Path.Combine(Path.GetTempPath(), $"step-cache-bypass-{System.Guid.NewGuid():N}.swb");
|
||||
try
|
||||
{
|
||||
Assert.Equal(1, cache.SaveToFile(path, map.MapID));
|
||||
|
||||
// Reset to a fresh state with the file open as a lazy reader and the deferred
|
||||
// promotion threshold restored to 2.
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 2;
|
||||
Assert.True(cache.TryOpenLazyReader(path, map.MapID));
|
||||
|
||||
// First touch must NOT return Fallthrough_NotBuilt — the lazy reader has the
|
||||
// chunk and serves it before the miss tracker is consulted.
|
||||
var lookup = cache.TryGetMask(map, 1500, 1600, sourceZ: 10);
|
||||
Assert.True(lookup.IsHit);
|
||||
Assert.Equal(CacheHitKind.Miss_NotBuilt, lookup.HitKind);
|
||||
Assert.Equal(1, cache.GetStats().ResidentChunks);
|
||||
Assert.Equal(0L, cache.GetStats().FallthroughNotBuilt);
|
||||
Assert.Equal(0L, cache.GetStats().BuildsTotal); // came from file, not baker
|
||||
}
|
||||
finally
|
||||
{
|
||||
cache.Clear();
|
||||
if (File.Exists(path))
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue