refactor: dedupe stats reduction (#15) + name the data-loss guard (#14)

openxr-api-layer/layer.cpp

@@ -137,6 +137,20 @@ namespace openxr_api_layer {
         std::atomic<uint32_t> g_skipNext{0};
         std::atomic<bool> 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-<pid>.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-<pid>.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-<pid>.csv "

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<double>& 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<MergedRow>& 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;
     }

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