From 9c11ccdb80cc68e2b35af60fbaefdf00e4f95dcb Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:07:20 -0700 Subject: [PATCH] fix(pathfinding): stop opening every .swb twice at boot (#2548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Every map's `.swb` step cache was opened, indexed and logged **twice** on boot. `MovementPath.Configure()` explicitly called `PathCacheCommands.Configure()` and `CacheEvictionTimer.Configure()`. Both are types exposing a public static parameterless `Configure()`, which `AssemblyHandler.Invoke("Configure")` already discovers and calls once each (`AssemblyHandler.cs:157`). So `PathCacheCommands.Configure()` ran twice, and `AutoLoadAtStartup()` with it. `PathCacheCommands.Configure()` called `PathfindRecorder.Configure()` the same way. `TryOpenLazyReader` disposes the prior reader before replacing it, so there was no handle leak — but the header and full chunk index of each `.swb` were read twice (~48 MB of files across six facets). The expensive `.mul` hashing was already memoized, so it was not doubled. ## Fix Consolidate the cache lifecycle into `Initialize`: - `Configure()` keeps only settings and command registration. - `Initialize()` opens the readers once, then prebakes only maps that still lack one. - The post-bake reopen is per-map instead of a blanket `AutoLoadAtStartup()` — on a partial bake (some valid `.swb`, one stale) that would close and reopen the readers already open, a second double-open on a different path. `Initialize` is the correct phase. `Configure` runs before `TileMatrixLoader.LoadTileMatrix()` and `World.Load()` (`Main.cs:458/460/463/465`), so opening a `.swb` there forced the lazy `Map.Tiles` property — the fingerprint hashes the map files — and built every `TileMatrix` ahead of the loader that owns it, possibly before `TileMatrix.Configure()` settled `Pre6000ClientSupport`. Both sit at the default call priority and the phase sort is unstable. Moving pathfinding out leaves nothing in `Configure` that touches `Map.Tiles`, closing that hazard; the other 22 `.Tiles` users in UOContent are all runtime paths. Multis stay out of the bake by design — houses and boats are player data that moves, handled by the multi-aware path at query time. ## Logging The per-map `StepCache: opened ... chunks indexed` line drops to `Debug`. Opening is the expected case; `BakeMap` already logs a rebuild at `Information`, and `Initialize` still emits `PathBake: pre-bake complete (N map(s) written)`. ## Verification - `dotnet build Projects/UOContent` — 0 errors, 0 warnings. - `dotnet test --filter FullyQualifiedName~Pathfinding` — **123 passed, 0 failed**. Boot logs should now show one `opened` line per map at `Debug`, none at `Information`. --- .../Engines/Pathing/Cache/StepCache.cs | 3 +- .../UOContent/Engines/Pathing/MovementPath.cs | 2 -- .../Engines/Pathing/PathCacheCommands.cs | 28 +++++++++++-------- dev-docs/pathfinding.md | 16 +++++++---- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Projects/UOContent/Engines/Pathing/Cache/StepCache.cs b/Projects/UOContent/Engines/Pathing/Cache/StepCache.cs index a1b368cb0..adf178927 100644 --- a/Projects/UOContent/Engines/Pathing/Cache/StepCache.cs +++ b/Projects/UOContent/Engines/Pathing/Cache/StepCache.cs @@ -285,7 +285,8 @@ public sealed class StepCache } _lazyReaders[mapId] = reader; - logger.Information( + // Debug: opening is the expected case. A rebuild is the interesting one, and BakeMap logs it. + logger.Debug( "StepCache: opened {Path} ({ChunkCount} chunks indexed) for map {MapId}", path, reader.IndexedChunkCount, mapId ); diff --git a/Projects/UOContent/Engines/Pathing/MovementPath.cs b/Projects/UOContent/Engines/Pathing/MovementPath.cs index b5307ad3d..32aa8ba11 100644 --- a/Projects/UOContent/Engines/Pathing/MovementPath.cs +++ b/Projects/UOContent/Engines/Pathing/MovementPath.cs @@ -60,8 +60,6 @@ public sealed class MovementPath public static void Configure() { CommandSystem.Register("Path", AccessLevel.GameMaster, Path_OnCommand); - CacheEvictionTimer.Configure(); - PathCacheCommands.Configure(); } [Usage("Path")] diff --git a/Projects/UOContent/Engines/Pathing/PathCacheCommands.cs b/Projects/UOContent/Engines/Pathing/PathCacheCommands.cs index f14886564..7f8fee270 100644 --- a/Projects/UOContent/Engines/Pathing/PathCacheCommands.cs +++ b/Projects/UOContent/Engines/Pathing/PathCacheCommands.cs @@ -39,15 +39,12 @@ public static class PathCacheCommands 8192 ); - PathfindRecorder.Configure(); - CommandSystem.Register("PathCacheStats", AccessLevel.Administrator, OnPathCacheStats); CommandSystem.Register("PathCacheClear", AccessLevel.Administrator, OnPathCacheClear); CommandSystem.Register("PathBake", AccessLevel.Administrator, OnPathBake); CommandSystem.Register("PathCacheSave", AccessLevel.Administrator, OnPathCacheSave); CommandSystem.Register("PathCacheLoad", AccessLevel.Administrator, OnPathCacheLoad); CommandSystem.Register("PathRecord", AccessLevel.Administrator, OnPathRecord); - AutoLoadAtStartup(); } /// @@ -80,18 +77,23 @@ public static class PathCacheCommands } /// - /// Bakes any map whose .swb is missing or stale, when is - /// set. Runs in the Initialize phase, once the tile matrix and world are loaded. An up-to-date - /// cache makes it a no-op, so the cost lands only on a first boot or after a client or map - /// update moves the fingerprint. + /// Opens the existing .swb files, then — when is set — + /// bakes any that are missing or stale. An up-to-date cache makes the bake a no-op, so the cost + /// lands only on a first boot or after a client or map update moves the fingerprint. /// - /// A map is judged up-to-date by whether it has an open reader. - /// already ran in the earlier Configure phase and only opens a reader for a .swb whose - /// fingerprint validates, so an open reader is proof of a good bake — no need to fingerprint - /// the map a second time here. + /// Both halves run here rather than in Configure: the fingerprint hashes the map files, so + /// opening a .swb forces the lazy property. In Configure that would + /// build every TileMatrix ahead of TileMatrixLoader, possibly before + /// TileMatrix.Configure() settles Pre6000ClientSupport — both sit at the default + /// call priority and the phase sort is unstable. + /// + /// A reader only opens once its fingerprint validates, so an open reader is proof of a good + /// bake and the map is skipped without fingerprinting it again. /// public static void Initialize() { + AutoLoadAtStartup(); + if (!ServerConfiguration.GetSetting(PrebakeSetting, false)) { return; @@ -119,13 +121,15 @@ public static class PathCacheCommands ); StepCache.Instance.BakeMap(map.MapID, path); StepCache.Instance.ClearResidentChunks(); + + // Just this map: a blanket AutoLoadAtStartup() would reopen every reader already open. + StepCache.Instance.TryOpenLazyReader(path, map.MapID); baked++; } if (baked > 0) { logger.Information("PathBake: pre-bake complete ({Count} map(s) written).", baked); - AutoLoadAtStartup(); // reopen what we just wrote } } diff --git a/dev-docs/pathfinding.md b/dev-docs/pathfinding.md index dfe3d8e09..861a41d75 100644 --- a/dev-docs/pathfinding.md +++ b/dev-docs/pathfinding.md @@ -139,13 +139,17 @@ several-minutes cost. Wiring: after assemblies load (so content can register prompts) but **before Serilog starts**, so the console prompt is not interleaved with the async console sink. Any class can participate by defining `public static void ConfigurePrompts()` and self-gating on first-boot state. -- The bake runs in the later `Invoke("Initialize")` phase (after the tile matrix + world load, - which the bake walks). +- Both the reader open and the bake run in `Invoke("Initialize")`, after the tile matrix and world + load. Neither belongs in `Configure`, which runs *before* both: the fingerprint hashes the map + files, so opening a `.swb` there would force the lazy `Map.Tiles` property and build every + `TileMatrix` ahead of `TileMatrixLoader` — possibly before `TileMatrix.Configure()` settles + `Pre6000ClientSupport`, since both sit at the default call priority and the phase sort is + unstable. `PathCacheCommands.Configure` is limited to settings and command registration. - Staleness is decided by the `.swb` fingerprint, which `StepCacheFile.OpenForLazy` validates at open time (hash of `tiledata.mul` + the per-map `.mul`/`.uop` files — never the in-memory - `TileData` tables, which the server patches at runtime). `Configure` opens a reader for every - up-to-date file; the bake in `Initialize` then skips any map where `StepCache.HasLazyReader` is - already true, so the fingerprint is computed once per boot, not twice. + `TileData` tables, which the server patches at runtime). `Initialize` opens a reader for every + up-to-date file, then skips any map where `StepCache.HasLazyReader` is already true, so the + fingerprint is computed once per boot. Each newly baked map reopens only itself. ## Configuration levers @@ -154,7 +158,7 @@ several-minutes cost. Wiring: | `pathfinding.enable` | `PathFollower.Configure` | `true` | Master switch for `PathFollower` pathfinding. Off → greedy/auto-turn only, no A* at all. | | `bitmap_pathfinding_cache` feature flag (`ContentFeatureFlags.BitmapPathfindingCache`, `Server.Systems.FeatureFlags`) | `FeatureFlagManager` | `true` | Off → `BitmapAStar` routes straight to the slow path with **no cache probe and no warming memory**. ≈ old FastAStar at ~1×. | | `pathfinding.maxResidentChunks` | `PathCacheCommands.Configure` | 8192 (~40 MB) | LRU cap on resident chunks = the warming-memory ceiling. Lower it (e.g. 512–1024 ≈ 2.5–5 MB) on small shards. | -| `pathfinding.maxSearchNodes` | `PathCacheCommands.Configure` → `BitmapAStarAlgorithm.MaxSearchNodes` | 1000 | A* per-Find node-expansion budget. See limits above; ~1000 is the sweet spot. | +| `pathfinding.maxSearchNodes` | `BitmapAStarAlgorithm.Configure` → `BitmapAStarAlgorithm.MaxSearchNodes` | 1000 | A* per-Find node-expansion budget. See limits above; ~1000 is the sweet spot. | | `pathfinding.prebakeMaps` | `PathCacheCommands` (first-boot prompt + `Initialize`) | `false` | When set, bakes any missing/stale `.swb` for the selected maps at startup (fingerprint-gated, so a fresh cache is a no-op). Set interactively by the first-boot prompt. | | `PathFollower` `RepathDelay` | `PathFollower.cs` (const) | 2 s | Throttle: a moving goal re-`Find`s at most ~once per 2 s; a stationary reachable goal is pathed once and reused until arrival. Not a setting (compile-time). |