fix(pipeline): MP0 - profiler toggle-path hygiene + Max() seed (review follow-up)
Three review finds, all in the runtime-toggle path or edge math (the steady-state path was confirmed correct): 1. GPU stale-slot across toggle-off/on: the disabled branch now Disposes the GpuFrameTimer (and nulls it) instead of Stop()-ing and keeping it — a kept instance would poll slots left pending from BEFORE the pause on re-enable and report temporally stale GPU samples. The re-enable branch's existing null guard rebuilds the ring fresh. Dispose is safe there: the branch runs at the top of OnRender with the GL context current. 2. Stale stage accumulation across mid-stage toggle-off: a StageScope disposed after the flag flips still calls EndStage, leaving a partial delta in _stageAccumTicks. The re-baseline branch now Array.Clear()s it alongside the GC re-baselining. 3. FrameStatsBuffer.Max() seeded with 0, clamping all-negative windows (the alloc channel can go negative if the boundary ever crosses threads) and disagreeing with Percentile(). Now seeds from the first live sample after the empty check (slots [0.._count) are always the live window regardless of ring wraparound); empty still returns 0 to match Percentile. TDD: Max_AllNegative_ReturnsTrueMax failed (returned 0) before the fix, passes after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
9f0da92ca7
commit
4b44a15286
3 changed files with 33 additions and 4 deletions
|
|
@ -75,7 +75,18 @@ public sealed class FrameProfiler : IDisposable
|
|||
bool enabled = RenderingDiagnostics.FrameProfEnabled;
|
||||
if (!enabled)
|
||||
{
|
||||
if (_wasEnabled) { _gpuTimer?.Stop(); _wasEnabled = false; _lastBoundaryTimestamp = 0; }
|
||||
if (_wasEnabled)
|
||||
{
|
||||
// Dispose (not just Stop) so a later re-enable rebuilds the
|
||||
// query ring fresh — a kept instance would poll slots left
|
||||
// pending from BEFORE the pause and report temporally stale
|
||||
// GPU samples. Safe here: this runs at the top of OnRender
|
||||
// with the GL context current.
|
||||
_gpuTimer?.Dispose();
|
||||
_gpuTimer = null;
|
||||
_wasEnabled = false;
|
||||
_lastBoundaryTimestamp = 0;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -92,9 +103,14 @@ public sealed class FrameProfiler : IDisposable
|
|||
if (!_wasEnabled)
|
||||
{
|
||||
// First enabled frame (startup or runtime toggle-on): establish
|
||||
// baselines, emit nothing.
|
||||
// baselines, emit nothing. Clear any stage ticks a StageScope
|
||||
// disposed after toggle-off may have accumulated mid-pause —
|
||||
// EndStage still runs on scopes that were live when the flag
|
||||
// flipped, and that partial delta must not leak into the first
|
||||
// re-enabled frame.
|
||||
_wasEnabled = true;
|
||||
_lastReportTicks = DateTime.UtcNow.Ticks;
|
||||
Array.Clear(_stageAccumTicks);
|
||||
_gc0Base = GC.CollectionCount(0); _gc1Base = GC.CollectionCount(1); _gc2Base = GC.CollectionCount(2);
|
||||
if (_gpuTimer is null && !_wbDiagActive) _gpuTimer = new GpuFrameTimer(gl);
|
||||
if (_wbDiagActive && !_wbDiagNoticePrinted)
|
||||
|
|
|
|||
|
|
@ -56,8 +56,13 @@ public sealed class FrameStatsBuffer
|
|||
|
||||
public long Max()
|
||||
{
|
||||
long max = 0;
|
||||
for (int i = 0; i < _count; i++)
|
||||
if (_count == 0) return 0; // documented empty behavior, matches Percentile
|
||||
// Seed from the first live sample so all-negative windows (the alloc
|
||||
// channel can go negative if the boundary ever crosses threads) return
|
||||
// the true max instead of clamping to 0. Slots [0.._count) are always
|
||||
// the live window regardless of ring wraparound.
|
||||
long max = _samples[0];
|
||||
for (int i = 1; i < _count; i++)
|
||||
if (_samples[i] > max) max = _samples[i];
|
||||
return max;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue