diff --git a/openxr-api-layer/layer.cpp b/openxr-api-layer/layer.cpp index 8e0eb49..a9388bb 100644 --- a/openxr-api-layer/layer.cpp +++ b/openxr-api-layer/layer.cpp @@ -137,6 +137,20 @@ namespace openxr_api_layer { std::atomic g_skipNext{0}; std::atomic g_monitoring{false}; + // THE single definition of "did the current session record any + // frames?" -- the data-loss invariant the merge relies on. + // MergeIntoOutput's zero-frame guard skips writing the merged CSV when + // this is false; FrameCsvSink's lazy-open is the per-side counterpart + // (it defers truncating the per-side CSV until the first row arrives). + // Together, a session that records nothing leaves every prior file on + // disk untouched. (The deeper fix -- per-session output filenames, so a + // no-op session physically cannot overwrite a prior one's files -- is + // noted in commit b643964 and deliberately deferred to keep one file + // per PID.) + bool SessionRecordedFrames() { + return g_frameCounter.load(std::memory_order_acquire) > 0; + } + // ---- Cross-DLL toggle sync ---------------------------------------- // // Naive per-DLL polling does NOT work for the sandwich: pre and @@ -894,17 +908,12 @@ namespace openxr_api_layer { const fs::path outCsv = localAppData / fmt::format("frames-merged-{}.csv", pid); - // ZERO-FRAME GUARD: a parasitic toggle (accidental Ctrl+F9 - // from another app fires the rising edge, user cancels with - // a second Ctrl+F9 before any xrEndFrame Append) MUST NOT - // remove the previous session's frames-merged-.csv - // from disk. With the FrameCsvSink lazy-open change, the - // per-side CSVs are also untouched in this case -- we mirror - // that by skipping the merge entirely here. g_frameCounter - // is reset to 0 on every ApplyToggle(true) and incremented - // only on real recorded frames, so g_frameCounter == 0 at - // toggle-OFF time means "this session never wrote a row". - if (g_frameCounter.load(std::memory_order_acquire) == 0) { + // ZERO-FRAME GUARD: a parasitic toggle (an accidental Ctrl+F9 from + // another app, cancelled by a second press before any xrEndFrame + // Append) recorded nothing, so skip the merge and leave the + // previous frames-merged-.csv untouched. See + // SessionRecordedFrames() above for the shared data-loss invariant. + if (!SessionRecordedFrames()) { Log("Skipping merge: this session recorded zero frames " "(parasitic toggle, or xrDestroyInstance fired before " "any xrEndFrame). Previous frames-merged-.csv " diff --git a/openxr-api-layer/utils/merge.cpp b/openxr-api-layer/utils/merge.cpp index 6f9dd2c..4320d13 100644 --- a/openxr-api-layer/utils/merge.cpp +++ b/openxr-api-layer/utils/merge.cpp @@ -312,6 +312,18 @@ namespace openxr_api_layer::merge { return sum + c; } + // Mean (Kahan-Neumaier compensated) + min + max over a NON-EMPTY + // vector, written into the three out-params. A single source for the + // reduction so a change to the summation strategy cannot drift between + // the ms / pct / gpu stat groups below (mirrored by analyze.py's + // _summarize, which keeps the merged-CSV header byte-equivalent). + void Summarize(const std::vector& v, double& mean, double& min, + double& max) { + mean = KahanNeumaierSum(v) / v.size(); + min = *std::min_element(v.begin(), v.end()); + max = *std::max_element(v.begin(), v.end()); + } + } // namespace MergeStats ComputeStats(const std::vector& merged) { @@ -340,27 +352,18 @@ namespace openxr_api_layer::merge { } } // ms_values always non-empty because merged is non-empty. - stats.target_ms_mean = KahanNeumaierSum(ms_values) / ms_values.size(); - stats.target_ms_min = *std::min_element(ms_values.begin(), ms_values.end()); - stats.target_ms_max = *std::max_element(ms_values.begin(), ms_values.end()); + Summarize(ms_values, stats.target_ms_mean, stats.target_ms_min, + stats.target_ms_max); if (!pct_values.empty()) { - stats.target_pct_mean = - KahanNeumaierSum(pct_values) / pct_values.size(); - stats.target_pct_min = - *std::min_element(pct_values.begin(), pct_values.end()); - stats.target_pct_max = - *std::max_element(pct_values.begin(), pct_values.end()); + Summarize(pct_values, stats.target_pct_mean, stats.target_pct_min, + stats.target_pct_max); } // GPU aggregates over frames that have a valid target_gpu_us. Stays // zero (and gpu_frame_count == 0) on CPU-only sessions. stats.gpu_frame_count = gpu_ms_values.size(); if (!gpu_ms_values.empty()) { - stats.target_gpu_ms_mean = - KahanNeumaierSum(gpu_ms_values) / gpu_ms_values.size(); - stats.target_gpu_ms_min = - *std::min_element(gpu_ms_values.begin(), gpu_ms_values.end()); - stats.target_gpu_ms_max = - *std::max_element(gpu_ms_values.begin(), gpu_ms_values.end()); + Summarize(gpu_ms_values, stats.target_gpu_ms_mean, + stats.target_gpu_ms_min, stats.target_gpu_ms_max); } return stats; } diff --git a/scripts/analyze.py b/scripts/analyze.py index 1c0a37d..2ca3791 100644 --- a/scripts/analyze.py +++ b/scripts/analyze.py @@ -82,6 +82,18 @@ def _parse_int4(raw: list[str]) -> tuple[int, int, int, int] | None: return None +def _summarize(values: list[float]) -> tuple[float, float, float]: + """(mean, min, max) over values, or (0.0, 0.0, 0.0) when empty. + + Mirrors merge.cpp's Summarize() -- a single source for the reduction so a + change cannot drift between the ms / pct / gpu stat groups, keeping the + merged-CSV header byte-equivalent regardless of which path produced it. + """ + if not values: + return (0.0, 0.0, 0.0) + return (statistics.fmean(values), min(values), max(values)) + + def load(path: Path) -> Frames: meta: dict[str, str] = {} rows: list[tuple[int, int, int, int]] = [] @@ -401,20 +413,14 @@ def main() -> int: # the last frame per thread (no successor -> no interval -> no pct), # matching the C++ merge. target_ms_values = [v / 1000.0 for v in target_values] - target_ms_mean = statistics.fmean(target_ms_values) if target_ms_values else 0.0 - target_ms_min = min(target_ms_values) if target_ms_values else 0.0 - target_ms_max = max(target_ms_values) if target_ms_values else 0.0 - target_pct_mean = statistics.fmean(pct_values) if pct_values else 0.0 - target_pct_min = min(pct_values) if pct_values else 0.0 - target_pct_max = max(pct_values) if pct_values else 0.0 + target_ms_mean, target_ms_min, target_ms_max = _summarize(target_ms_values) + target_pct_mean, target_pct_min, target_pct_max = _summarize(pct_values) # GPU aggregates over frames with a valid target_gpu_us. gpu_frame_count # == 0 means GPU was not captured (non-D3D11 host); the ms_* lines are # then 0.0000 and every target_gpu_us cell is blank. Matches C++ ComputeStats. gpu_ms_values = [v / 1000.0 for v in gpu_values] - target_gpu_ms_mean = statistics.fmean(gpu_ms_values) if gpu_ms_values else 0.0 - target_gpu_ms_min = min(gpu_ms_values) if gpu_ms_values else 0.0 - target_gpu_ms_max = max(gpu_ms_values) if gpu_ms_values else 0.0 + target_gpu_ms_mean, target_gpu_ms_min, target_gpu_ms_max = _summarize(gpu_ms_values) # newline="" disables the file object's translation; lineterminator='\n' # disables csv.writer's default '\r\n'. Together they keep the output