performance: keep results measured before a history reset out of the series

doc/benchmarks.md

@@ -736,6 +736,8 @@ host doesn't record, such as a performance tier or bursting setting, reset at le
 affected metrics by removing them from the series' history files, such as
 `data/windows-whp-virtual-machine-microvm-v2-1vcpu.csv`, so that the gate restarts
 their ten-point warmup instead of comparing the new storage with the old baseline.
+Results measured before such a reset commit are not persisted into the reset
+series, so a `dev` run that finishes after the reset cannot refill it.
 
 The regression gate compares the target p50 with the median of the latest 10
 p50 values on the pull request's base branch and requires all 10

scripts/nvx_tools/performance.py

@@ -9,9 +9,10 @@
 import os
 import re
 import statistics
+import subprocess
 import sys
 import tempfile
-from collections import defaultdict, deque
+from collections import Counter, defaultdict, deque
 from collections.abc import Callable, Iterable, Sequence
 from dataclasses import dataclass, replace
 from pathlib import Path
@@ -2248,11 +2249,161 @@ def _verify_result_hosts(
             )
 
 
+def _git(checkout: Path, *args: str) -> str:
+    completed = subprocess.run(
+        ["git", "-C", str(checkout), *args],
+        check=False,
+        capture_output=True,
+        text=True,
+    )
+    if completed.returncode != 0:
+        raise PerformanceError(
+            f"git {' '.join(args)} failed: {completed.stderr.strip()}"
+        )
+    return completed.stdout
+
+
+def _committed_results(top: Path, revision: str, relative: str) -> list[Result]:
+    shown = subprocess.run(
+        ["git", "-C", str(top), "show", f"{revision}:{relative}"],
+        check=False,
+        capture_output=True,
+    )
+    if shown.returncode != 0:
+        return []
+    with tempfile.TemporaryDirectory() as temporary:
+        # Keep the basename: a legacy CSV takes its platform from the file name.
+        path = Path(temporary) / Path(relative).name
+        path.write_bytes(shown.stdout)
+        return read_results(path)
+
+
+def _dimension_counts(
+    top: Path,
+    revision: str,
+    relative: str,
+) -> Counter[tuple[str, int, int, str]]:
+    return Counter(
+        _dimension_key(result, "")
+        for result in _committed_results(top, revision, relative)
+    )
+
+
+def _reset_dimensions_since(
+    git_checkout: Path,
+    history_path: Path,
+    commit: str,
+) -> set[tuple[str, int, int, str]]:
+    """Return the dimensions whose committed history lost rows after ``commit``.
+
+    A result measured at ``commit`` predates any later commit that removes the
+    dimension's rows, so it must not be appended onto the reset series.
+    """
+    top = Path(_git(git_checkout, "rev-parse", "--show-toplevel").strip()).resolve()
+    try:
+        relative = history_path.resolve().relative_to(top).as_posix()
+    except ValueError as error:
+        raise PerformanceError(f"{history_path} is outside the git checkout") from error
+    ancestor = subprocess.run(
+        ["git", "-C", str(top), "merge-base", "--is-ancestor", commit, "HEAD"],
+        check=False,
+        capture_output=True,
+    )
+    if ancestor.returncode != 0:
+        raise PerformanceError(
+            f"result commit {commit} is not an ancestor of the target history"
+        )
+    # Compare each commit with its own parents: a side branch forked before
+    # ``commit`` can hold fewer rows than ``commit`` without having reset them.
+    later = _git(
+        top,
+        "log",
+        "--reverse",
+        "--full-history",
+        "--sparse",
+        "--format=%H %P",
+        f"{commit}..HEAD",
+        "--",
+        relative,
+    ).splitlines()
+    resets: set[tuple[str, int, int, str]] = set()
+    for line in later:
+        revision, *parents = line.split()
+        # Ignore commits on side branches that are not part of the history
+        # leading from the measurement to HEAD, even if later merged/reverted.
+        if not _on_measurement_line(top, revision, commit):
+            continue
+        current = _dimension_counts(top, revision, relative)
+        for parent in parents:
+            # A branch that neither predates nor follows the measurement can drop rows
+            # that were already reset before it, so only the measurement's line counts.
+            if not _on_measurement_line(top, parent, commit):
+                continue
+            before = _dimension_counts(top, parent, relative)
+            resets.update(key for key, count in before.items() if current[key] < count)
+    return resets
+
+
+def _on_measurement_line(top: Path, revision: str, measured: str) -> bool:
+    """Return whether ``revision`` is on the path from ``measured`` to HEAD."""
+    return all(
+        subprocess.run(
+            [
+                "git",
+                "-C",
+                str(top),
+                "merge-base",
+                "--is-ancestor",
+                ancestor,
+                descendant,
+            ],
+            check=False,
+            capture_output=True,
+        ).returncode
+        == 0
+        for ancestor, descendant in ((measured, revision), (revision, "HEAD"))
+    )
+
+
+def _without_reset_rows(
+    git_checkout: Path,
+    history_path: Path,
+    results: Sequence[Result],
+) -> list[Result]:
+    resets: dict[str, set[tuple[str, int, int, str]]] = {}
+    kept: list[Result] = []
+    for result in results:
+        if result.commit not in resets:
+            resets[result.commit] = _reset_dimensions_since(
+                git_checkout, history_path, result.commit
+            )
+        if _dimension_key(result, "") in resets[result.commit]:
+            print(
+                f"Skipped {result.metric} measured at {result.commit[:12]}: "
+                f"{history_path.name} was reset after that commit"
+            )
+            continue
+        kept.append(result)
+    return kept
+
+
+def _git_checkout_of(path: Path) -> Path | None:
+    """Return ``path`` when it lies inside a git checkout, which CI always does."""
+    probe = subprocess.run(
+        ["git", "-C", str(path), "rev-parse", "--is-inside-work-tree"],
+        check=False,
+        capture_output=True,
+        text=True,
+    )
+    return path if probe.returncode == 0 and probe.stdout.strip() == "true" else None
+
+
 def persist_results(
     source_dir: Path,
     history_dir: Path,
     excluded_metrics: Iterable[str] = (),
     require_host: bool = False,
+    git_checkout: Path | None = None,
 ) -> None:
     source_files = sorted(source_dir.glob("*.csv"))
     if not source_files:
@@ -2275,6 +2426,14 @@ def persist_results(
         current = [result for result in source_results if result.metric not in excluded]
         history_path = history_dir / source_path.name
         existing = read_results(history_path) if history_path.exists() else []
+        existing_keys = {_result_identity(result) for result in existing}
+        current = [
+            result
+            for result in current
+            if _result_identity(result) not in existing_keys
+        ]
+        if git_checkout is not None:
+            current = _without_reset_rows(git_checkout, history_path, current)
         metadata = {
             (
                 result.platform,
@@ -2300,12 +2459,7 @@ def persist_results(
                     f"{expected} -> {actual}"
                 )
 
-        existing_keys = {_result_identity(result) for result in existing}
-        new_results = [
-            result
-            for result in current
-            if _result_identity(result) not in existing_keys
-        ]
+        new_results = current
         if not new_results:
             print(f"No new performance rows to persist for {source_path.name}")
             continue
@@ -2916,6 +3070,7 @@ def command_performance(args: argparse.Namespace) -> int:
             args.history_dir,
             args.exclude_metric,
             require_host=args.require_host,
+            git_checkout=_git_checkout_of(args.history_dir),
         )
         return 0
     except (PerformanceError, OSError, csv.Error) as error: