From 11f0c1335b9226617c9333a63d042ad65dac06f7 Mon Sep 17 00:00:00 2001 From: Fr0zka Date: Mon, 14 Sep 2026 05:17:27 +0200 Subject: [PATCH] fix(diagnostics): sampler can no longer crash the run it measures; worm skip default off Harness round. Every launched run is in Saved/HarnessFixAcceptance_20260914.ledger.txt. - FVoxelStackSampler: two sampler-on runs had died in EndPlay (logs stop after "workers drained", before the latency summary). The lifecycle teardown is now serialised, registration TLS is tagged by session, stale worker registrations are cleared safely, and in-process DbgHelp initialisation/symbolisation is REMOVED. The measured process now writes raw PCs only; symbol resolution is deferred to after process exit. (An offline symboliser is still needed for readable tables; it is queued first in the next round.) - voxel.WormBlockSkip default 1 -> 0: it measured neutral (26b3476). The code, the proven bound and the soundness test are kept for a tighter skip later. - Harness (Saved/*.ps1, not tracked): a writable DDC under the plugin's Saved\DDC with -ddc=NoZenLocalFallback, fixing the DerivedDataBackends.cpp:208 startup fatal. UE 5.7 Windows was verified NOT to honour -nocrashreports for its startup monitor, so the harness runs unattended with dialogs forced off plus a PID-scoped CrashReportClient watchdog. Failures still exit non-zero, and a run without its final summary line is rejected. Acceptance: 10 consecutive mixed runs, 7 of them sampler-on, all PASS; no new crash folders, dumps, dialogs or lingering processes. Other runs: one smoke run failed on a PowerShell HashSet.ToArray() compatibility bug in the harness (Unreal itself exited cleanly); its retries and the export passed. Export OBJ unchanged (b3e5f4c3..9b377). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TaFdXhTdYMkW1xrV6ApT6Y --- CODEMAP.md | 4 +- Source/VoxelForge/Private/VoxelGenerator.cpp | 4 +- .../VoxelForge/Private/VoxelStackSampler.cpp | 129 ++++++++++++++++-- Source/VoxelForge/Public/VoxelStackSampler.h | 14 +- 4 files changed, 127 insertions(+), 24 deletions(-) diff --git a/CODEMAP.md b/CODEMAP.md index dff9a56..4cd7c68 100644 --- a/CODEMAP.md +++ b/CODEMAP.md @@ -301,7 +301,7 @@ casse cette arithmetique *structurellement* — `BeginPlay` loggue une **Error** | `GetPlayerPosition` | 231 | Pawn position or zero. (`GetLODForChunk`/`LODToStep`/`IsChunkInRange` removed — dead since the clipmap.) | | `ProcessPendingChunks` | 301 | Drains ProcessQueue under per-frame budget; reads the per-request cancellation token before removing its map entry, drops obsolete/aborted results, then applies each live result via `ApplyTileResult`. | | `ApplyTileResult` | — | **Shared game-thread apply** for one `FChunkResult` (async drain + sync carve): discards stale generation epochs and keys no longer desired, marks loaded, ingests capture, EMPTY releases the tile's existing component (a re-gen can flip content→empty on band change — old geometry must not linger), else `ApplyMeshToTile`. Returns true iff a visible mesh uploaded (counts the budget). Doesn't touch `PendingTiles` (caller's). | -| `FVoxelStackSampler` / `FScopedVoxelStackRegistration` (`VoxelStackSampler.h/.cpp`) | — | **Diagnostic stack-sampling profiler (2026-09-13), off by default.** A sampler thread captures registered generation workers' stacks (`CaptureThreadStackBackTrace`) every N µs and tags each with the tile's LOD. It symbolises after the run (inline frames too) into CSV plus a summary under `Saved/`. Registration is per tile TASK in `LoadTile` (and the export's tasks), never per sample; disabled costs one relaxed atomic load. Game `-voxel.SampleStacks=`, commandlet `-samplestacks[=us]`. ~4-5% overhead at 1 ms, so trust large shares, not single-digit deltas. Report: `PERF-SAMPLED.md`. | +| `FVoxelStackSampler` / `FScopedVoxelStackRegistration` (`VoxelStackSampler.h/.cpp`) | — | **Diagnostic raw stack sampler, off by default.** A sampler thread captures registered generation workers' stacks (`CaptureThreadStackBackTrace`) every N µs and tags each with the tile's LOD. It writes bounded raw PCs to CSV plus a symbolization-free summary under `Saved/`; symbol resolution is deferred until after process exit so the measured process never initializes/calls DbgHelp for this diagnostic. Lifecycle teardown is serialized and registration TLS is session-tagged. Registration is per tile TASK in `LoadTile` (and the export's tasks), never per sample; disabled costs one relaxed atomic load. Game `-voxel.SampleStacks=`, commandlet `-samplestacks[=us]`. | | Room-cache performance (2026-09-13): `VF_FindPlayerFitPointForRoomMemoized` / `GPlayerFitMemo` (`VoxelCaveMorphology.cpp`), `voxel.TileCacheWindow` + `VoxelGenLOD::TileOriginVoxels/TileCellsPerAxis` (`VoxelGenerator.h`), `FChunkSDFSpatialIndex` | — | **Player-fit memo:** exact 97-word key, process-wide, `voxel.PlayerFitMemo`. **Tile cache window:** the room cache covers the requesting tile's footprint plus a fixed collect margin. It is set via `TGuardValue` in `GenerateMesh`/classify, and is window-invariant by construction (the margin does not grow with the tile). **LOD-gated:** fused path at LOD ≥ `voxel.TileCacheWindowMinLOD` (3), op-stack at LOD ≥ `voxel.TileCacheWindowOpStackMinLOD` (4); lower LODs keep the 4-chunk worker window, because the tile window slowed LOD0 evaluation ~6%. Strates whose authored terrain ops exceed the proven 256-voxel extent (or are malformed/unloaded) fall back to the legacy window (`VF_ComputeTerrainOpEnvelope`). `voxel.SpatialIndex` is -1 auto (off at fused LOD0, on elsewhere), 0 or 1. **Spatial index:** ordered XY buckets over cache primitives, exact rejection. The `static_assert` in `VoxelStrateTypes.h` ties `VF_STRATE_PARAM_FIELDS` to `FStrateGenerationParams` (83 fields + 8 B padding). | | `GenerateTileResult` | — | **Shared worker-side gen** for one tile (async `LoadTile` task + sync `SyncRemeshTile`): [ClassifyTile (T1.d) + exact validation, only when `voxel.OuterClassifierMode=1`; **default 0 since 2026-09-13**] → `GenerateMesh`/`GenerateSheetMesh` → `BuildTileStreamSet`. Reads Generator/Mesher only → safe on a worker or the game thread; fills `FChunkResult`, no enqueue. | | `SyncRemeshTile` | — | **INSTANT DIG**: level-0 same-frame re-mesh on the game thread (`GenerateTileResult` + `ApplyTileResult` inline, Cells=CHUNK_SIZE/Step=1 + strate band, no capture). Used for the tile under the brush centre so a carve is visible THIS frame; one full-res gen on the game thread. | @@ -341,7 +341,7 @@ This is **where terrain shape lives.** | `ApplyPassageCarving` (static) | 197 | Punches passages/elevator through the seal. | | `InitializeSettings` | 211 | Copies the effective seed, spine radius, and global XY world-bound settings from `UVoxelSettings` (the season owns all three when assigned); radius 0 preserves the unbounded field. | | **`GetDensityAt`** | 218 | **Entry point.** Picks strate + generator type, materialises a cooked-season recipe on the existing per-chunk refetch when present, dispatches, then adds disturbances/diff. Its `CP_*` state is keyed by `(DensityCacheOwnerId, ChunkCoord, LayoutVersion)`; workers copy immutable recipe/vector data and never read editor composer state. | -| **`GetDensityWithParams`** | 277 | TunnelNetwork pipeline (~1000 lines). See §4. ⚠️ Takes **required** `ParamsFingerprint` + `LayoutVersion` since the AUDIT §C2 fix (2026-07-28) — they go into the SDF cache key so a chunk can no longer be evaluated against a neighbour's rooms. Callers compute the CRC **once per chunk** (`CP_TunnelFP`), never per voxel. The optional `voxel.WormBlockSkip` path uses `VF_TryGetWormBlockSkip` + a worker-local 4³ lattice-block cache to prove `|N1| >= WormThreshold` and remove both worm noise calls only; it never changes `NetworkMask` or the fallback field path. | +| **`GetDensityWithParams`** | 277 | TunnelNetwork pipeline (~1000 lines). See §4. ⚠️ Takes **required** `ParamsFingerprint` + `LayoutVersion` since the AUDIT §C2 fix (2026-07-28) — they go into the SDF cache key so a chunk can no longer be evaluated against a neighbour's rooms. Callers compute the CRC **once per chunk** (`CP_TunnelFP`), never per voxel. The optional `voxel.WormBlockSkip` path (default 0) uses `VF_TryGetWormBlockSkip` + a worker-local 4³ lattice-block cache to prove `|N1| >= WormThreshold` and remove both worm noise calls only; it never changes `NetworkMask` or the fallback field path. | | **`GetSlabDensity`** | 1306 | FlatPlain/CrystalChamber pipeline. See §4.2. | | `SampleSurfaceStructuralZ` | — | **F20:** the RAW SurfaceWorld heightfield (continents+mountains+detail), BEFORE any terrain op; returns terrain Z + relief M. Cliff re-samples it at an XY offset for a cheap analytic slope. | | `ComputeSurfaceTerrainZ` / `GetSurfaceDensity` | — | SurfaceWorld heightfield → terrain Z, then density; biome **output-blend** lerps dominant/neighbour heights (`ParamsD`/`ParamsN`/weight). **F20 surface ops** (`FSurfaceGenerationParams`, biome-selected + slope/relief-conditioned, all default off): Cliff (slope-gated STEEPENING — push height from local mean where steep ⇒ sheer walls; 4 structural resamples only when on), Terrace (relief-gated + `TerraceHardness`), LayerLines (sedimentary shelves) — pure per-column height REMAPS applied here so the single height oracle stays consistent (MC/sheets/ClassifyTile/deco/BP bridge). **Phase 2 OVERHANG** (volumetric — real jutting shelves): in `SurfaceDensityFromColumn`, for AIR voxels in a window `(TerrainZ, TerrainZ+OverhangHeight]` above a steep slope, the heightfield is re-sampled UPHILL (toward the cliff) by a reach that GROWS with height (tiny low ⇒ air over the void, full high ⇒ borrows the far cliff rock) and unioned in ⇒ a shelf attached to the cliff, tapering out over the void with air beneath (the sketch). Per-column `OverhangAmp`(=strength·slope-gate) + unit uphill `(DirX,DirY)` resolved once in `ComputeSurfaceColumn` (gradient sampled at the REACH scale so a spot over the void can see the cliff), cached on `FSurfaceColumn`. Genuine 3D (per-voxel structural re-eval, gated to steep overhang columns). Off ⇒ byte-identical. The field-preserving LOD0 pass removes only needless temporaries/default construction; every output remains bit-identical. §8.14. | diff --git a/Source/VoxelForge/Private/VoxelGenerator.cpp b/Source/VoxelForge/Private/VoxelGenerator.cpp index 8ddfb2e..baa0002 100644 --- a/Source/VoxelForge/Private/VoxelGenerator.cpp +++ b/Source/VoxelForge/Private/VoxelGenerator.cpp @@ -87,8 +87,8 @@ namespace // A proved N1-above-threshold block skips both worm noise calls. The switch is parsed once // on the same scalar entry point as the other generation A/B switches; 0 is the exact legacy - // path, and 1 is the default after the proof/soundness test passes. - int32 GVoxelForgeWormBlockSkip = 1; + // path and remains the default until a tighter skip is justified by a future measurement. + int32 GVoxelForgeWormBlockSkip = 0; FAutoConsoleVariableRef CVarVoxelForgeWormBlockSkip( TEXT("voxel.WormBlockSkip"), GVoxelForgeWormBlockSkip, diff --git a/Source/VoxelForge/Private/VoxelStackSampler.cpp b/Source/VoxelForge/Private/VoxelStackSampler.cpp index 339c555..f2badcc 100644 --- a/Source/VoxelForge/Private/VoxelStackSampler.cpp +++ b/Source/VoxelForge/Private/VoxelStackSampler.cpp @@ -1,5 +1,5 @@ // VoxelStackSampler.cpp -// Fixed-storage sampling and post-run symbolization for VoxelForge generation workers. +// Fixed-storage raw stack sampling for VoxelForge generation workers. #include "VoxelStackSampler.h" @@ -18,6 +18,7 @@ #include "Containers/StringConv.h" #include "Misc/FileHelper.h" #include "Misc/Paths.h" +#include "Misc/ScopeLock.h" #include "Serialization/Archive.h" #include "Templates/UniquePtr.h" @@ -49,6 +50,20 @@ thread_local int32 GTlsRegisteredSlot = INDEX_NONE; thread_local uint64 GTlsRegistrationGeneration = 0; thread_local int32 GTlsRegistrationDepth = 0; thread_local int32 GTlsRegisteredLODLevel = INDEX_NONE; +thread_local void* GTlsRegisteredImpl = nullptr; +thread_local uint64 GTlsSamplerSessionGeneration = 0; + +std::atomic GNextSamplerSessionGeneration(1); + +void ResetRegistrationTls() +{ + GTlsRegisteredSlot = INDEX_NONE; + GTlsRegistrationGeneration = 0; + GTlsRegistrationDepth = 0; + GTlsRegisteredLODLevel = INDEX_NONE; + GTlsRegisteredImpl = nullptr; + GTlsSamplerSessionGeneration = 0; +} FString SanitizeLabel(const FString& InLabel) { @@ -293,7 +308,7 @@ void AppendRows( struct FVoxelStackSampler::FImpl final : public FRunnable { -struct FStoredSample + struct FStoredSample { uint64 ElapsedUs = 0; uint64 ThreadId = 0; @@ -310,6 +325,7 @@ struct FStoredSample FString SummaryPath; double StartSeconds = 0.0; std::atomic bStop{false}; + uint64 SamplerSessionGeneration = 0; std::atomic NextRegistrationGeneration{1}; FRegisteredThreadSlot Slots[MaxRegisteredThreads]; TUniquePtr Samples; @@ -337,6 +353,8 @@ struct FStoredSample , RunLabel(InRunLabel) , OutputDirectory(InOutputDirectory) { + SamplerSessionGeneration = GNextSamplerSessionGeneration.fetch_add( + 1, std::memory_order_relaxed); const FString SafeLabel = SanitizeLabel(RunLabel); const uint32 ProcessId = FPlatformProcess::GetCurrentProcessId(); RawSamplesPath = FPaths::Combine( @@ -390,7 +408,9 @@ struct FStoredSample bool StartThread() { #if PLATFORM_WINDOWS - bStackWalkingInitialized = FPlatformStackWalk::InitStackWalking(); + // CaptureThreadStackBackTrace uses the same-process context path and deliberately does + // not require DbgHelp. Initializing DbgHelp here made the measured process depend on + // global symbol-engine state before shutdown; symbolization is an offline concern. StartSeconds = FPlatformTime::Seconds(); #endif Thread = FRunnableThread::Create(this, TEXT("VoxelStackSampler"), 0, TPri_Lowest); @@ -414,8 +434,16 @@ struct FStoredSample const uint32 CurrentThreadId = FPlatformTLS::GetCurrentThreadId(); if (GTlsRegistrationDepth > 0) { - ++GTlsRegistrationDepth; - return true; + if (GTlsRegisteredImpl == this + && GTlsSamplerSessionGeneration == SamplerSessionGeneration) + { + ++GTlsRegistrationDepth; + return true; + } + + // A worker can outlive a session's owner-side shutdown. Never let stale TLS from + // that session masquerade as a nested registration in a later session. + ResetRegistrationTls(); } const uint64 Generation = NextRegistrationGeneration.fetch_add( @@ -439,6 +467,8 @@ struct FStoredSample GTlsRegistrationGeneration = Generation; GTlsRegistrationDepth = 1; GTlsRegisteredLODLevel = LODLevel; + GTlsRegisteredImpl = this; + GTlsSamplerSessionGeneration = SamplerSessionGeneration; return true; } return false; @@ -467,9 +497,7 @@ struct FStoredSample Slot.LODLevel.store(INDEX_NONE, std::memory_order_relaxed); } } - GTlsRegisteredSlot = INDEX_NONE; - GTlsRegistrationGeneration = 0; - GTlsRegisteredLODLevel = INDEX_NONE; + ResetRegistrationTls(); } void CaptureRegisteredThreads(double NowSeconds) @@ -598,7 +626,10 @@ struct FStoredSample Result.RunSeconds = FPlatformTime::Seconds() - StartSeconds; WriteRawSamples(Result); - SymbolizeAndWriteSummary(Result); + // Do not call the DbgHelp-backed symbolizer from the measured process. DbgHelp is a + // process-global state machine and can fault during shutdown even after the sampler + // thread has joined. The raw PCs remain available for a separate offline pass. + WriteRawOnlySummary(Result); return Result; } @@ -641,6 +672,61 @@ struct FStoredSample Result.bOutputWritten = true; } + void WriteRawOnlySummary(FSummary& Result) + { + uint64 SamplesWithStack = 0; + for (uint32 SampleIndex = 0; SampleIndex < StoredSampleCount; ++SampleIndex) + { + if (Samples[SampleIndex].Depth > 0) + { + ++SamplesWithStack; + } + } + + FString Summary; + Summary += TEXT("VoxelForge raw stack sampler\n"); + Summary += TEXT("============================\n"); + Summary.Appendf(TEXT("run_label: %s\n"), *RunLabel); + Summary.Appendf(TEXT("supported: %s\n"), Result.bSupported ? TEXT("yes") : TEXT("no")); + Summary += TEXT("raw_only: yes\n"); + Summary += TEXT("symbolization: deferred\n"); + Summary += TEXT("dbghelp_in_measured_process: no\n"); + Summary.Appendf(TEXT("interval_us: %u\n"), IntervalUs); + Summary.Appendf(TEXT("max_stack_depth: %u\n"), MaxStackDepth); + Summary.Appendf(TEXT("max_stored_samples: %u\n"), MaxStoredSamples); + Summary.Appendf(TEXT("sampler_ticks: %llu\n"), + static_cast(SamplerTicks)); + Summary.Appendf(TEXT("capture_attempts: %llu\n"), + static_cast(CaptureAttempts)); + Summary.Appendf(TEXT("registered_thread_samples: %llu\n"), + static_cast(RegisteredThreadSamples)); + Summary.Appendf(TEXT("registration_race_drops: %llu\n"), + static_cast(RegistrationRaceDrops)); + Summary.Appendf(TEXT("capture_failures: %llu\n"), + static_cast(CaptureFailures)); + Summary.Appendf(TEXT("retained_samples: %llu\n"), + static_cast(StoredSampleCount)); + Summary.Appendf(TEXT("dropped_samples_after_bound: %llu\n"), + static_cast(DroppedSamples)); + Summary.Appendf(TEXT("samples_with_stack: %llu\n"), + static_cast(SamplesWithStack)); + Summary.Appendf(TEXT("total_frames: %llu\n"), + static_cast(TotalFrames)); + Summary.Appendf(TEXT("max_active_registered_threads: %llu\n"), + static_cast(MaxActiveRegisteredThreads)); + Summary.Appendf(TEXT("run_seconds: %.6f\n"), Result.RunSeconds); + Summary.Appendf(TEXT("raw_samples_path: %s\n"), *RawSamplesPath); + Summary += TEXT("\nThe target process records bounded raw PCs only; resolve symbols after the process exits.\n"); + + if (FFileHelper::SaveStringToFile( + Summary, + *SummaryPath, + FFileHelper::EEncodingOptions::ForceUTF8WithoutBOM)) + { + Result.bOutputWritten = true; + } + } + void ResolveProgramCounter( uint64 ProgramCounter, TMap& SymbolCache, @@ -948,6 +1034,7 @@ bool FVoxelStackSampler::Start( const FString& OutputDirectory, const FString& RunLabel) { + FScopeLock LifecycleLock(&LifecycleMutex); if (Impl != nullptr) { return false; @@ -982,6 +1069,7 @@ bool FVoxelStackSampler::Start( FVoxelStackSampler::FSummary FVoxelStackSampler::StopAndWrite() { GVoxelStackSamplerEnabled.store(false, std::memory_order_release); + FScopeLock LifecycleLock(&LifecycleMutex); FSummary Result; if (Impl == nullptr) { @@ -1011,14 +1099,12 @@ FVoxelStackSampler::FSummary FVoxelStackSampler::StopAndWrite() void FVoxelStackSampler::Shutdown() { - if (Impl != nullptr) - { - StopAndWrite(); - } + StopAndWrite(); } bool FVoxelStackSampler::IsRunning() const { + FScopeLock LifecycleLock(&LifecycleMutex); return Impl != nullptr; } @@ -1045,13 +1131,28 @@ bool FVoxelStackSampler::IsEnabled() bool FVoxelStackSampler::RegisterThreadInternal(int32 LODLevel) { + FScopeLock LifecycleLock(&LifecycleMutex); return Impl != nullptr && Impl->RegisterThread(LODLevel); } void FVoxelStackSampler::DeregisterThreadInternal() { - if (Impl != nullptr) + FScopeLock LifecycleLock(&LifecycleMutex); + if (GTlsRegistrationDepth <= 0) + { + ResetRegistrationTls(); + return; + } + + if (Impl != nullptr + && GTlsRegisteredImpl == Impl + && GTlsSamplerSessionGeneration == Impl->SamplerSessionGeneration) { Impl->DeregisterThread(); + return; } + + // StopAndWrite may have detached and destroyed the session before a worker's scope guard + // runs. Clear only this thread's stale registration; never dereference the old FImpl. + ResetRegistrationTls(); } diff --git a/Source/VoxelForge/Public/VoxelStackSampler.h b/Source/VoxelForge/Public/VoxelStackSampler.h index 54415c5..621e561 100644 --- a/Source/VoxelForge/Public/VoxelStackSampler.h +++ b/Source/VoxelForge/Public/VoxelStackSampler.h @@ -1,21 +1,22 @@ // VoxelStackSampler.h -// Opt-in, out-of-process-symbolized stack sampling for active VoxelForge generation workers. +// Opt-in raw stack sampling for active VoxelForge generation workers; symbols are resolved offline. #pragma once #include "CoreTypes.h" #include "Containers/UnrealString.h" +#include "HAL/CriticalSection.h" /** - * A diagnostic-only in-process stack sampler. + * A diagnostic-only raw stack sampler. * * The generation path only constructs FScopedVoxelStackRegistration at task entry. When the * sampler is disabled that constructor performs one relaxed load of the global enable flag and * returns; it does not touch the registry, allocate, lock, or read a console variable. * - * The sampler thread owns a bounded preallocated array of raw program counters. Symbolization - * and aggregation happen after the sampler thread has stopped, so DbgHelp and FString activity are - * kept out of both generation workers and the sampling loop. + * The sampler thread owns a bounded preallocated array of raw program counters. The measured + * process never initializes or calls DbgHelp: the raw CSV is the durable sampling artifact and + * symbolization is explicitly deferred to an offline consumer. */ class VOXELFORGE_API FVoxelStackSampler { @@ -66,7 +67,7 @@ public: /** Start one sampling session. Returns false when another session is active or setup fails. */ bool Start(uint32 IntervalUs, const FString& OutputDirectory, const FString& RunLabel); - /** Stop, symbolize, aggregate, and write the bounded raw CSV plus readable summary. */ + /** Stop and write the bounded raw CSV plus a symbolization-free readable summary. */ FSummary StopAndWrite(); /** Used by the module shutdown path as a final lifecycle backstop. */ @@ -87,6 +88,7 @@ private: struct FImpl; FImpl* Impl = nullptr; + mutable FCriticalSection LifecycleMutex; bool RegisterThreadInternal(int32 LODLevel); void DeregisterThreadInternal();