From 540e463c9963fdeeae759d1407e0a2adedfd62f8 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 14 Oct 2026 15:32:44 +0530 Subject: [PATCH] CHORE: Improve PR performance sensitivity and measurement reliability Detect consistent small and large changes without promoting isolated jitter into performance signals. Warm steady-state workloads in each measurement process and amortize short timings with fixed, comparable repetition blocks while retaining per-workload impact and diagnostic units. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eng/profiler_benchmarks/README.md | 32 +++- eng/profiler_benchmarks/controller.py | 56 +++++- eng/profiler_benchmarks/report.py | 171 +++++++++-------- eng/profiler_benchmarks/workloads.py | 11 ++ tests/test_036_profiler_ci.py | 252 ++++++++++++++++++++++++-- 5 files changed, 417 insertions(+), 105 deletions(-) diff --git a/eng/profiler_benchmarks/README.md b/eng/profiler_benchmarks/README.md index 2fdeb4d91..53356f347 100644 --- a/eng/profiler_benchmarks/README.md +++ b/eng/profiler_benchmarks/README.md @@ -30,7 +30,23 @@ improvement thresholds are unchanged. CI uses the PR merge's first parent as the exact base. It reuses the profiling-enabled candidate build after pytest and builds the base separately. -Fresh processes run five measured pairs after one warmup pair in alternating order. +Fresh processes run five measured pairs after one discarded warmup pair in alternating order. +Each worker also warms every steady-state task once before measuring it in that +same process. Connection opening remains a single cold invocation. + +Short read/query tasks use fixed blocks shared by both revisions: SELECT and +256 KiB LOB fetching repeat 100 times, row-by-row fetching 10 times, CTE queries +20 times, and positional/named query workloads 3 times. On the observed hosted +Unix controls these blocks target roughly 100 ms or more; they are not adaptive +duration gates. Other workloads remain single-pass. Setup and cleanup keep their +existing timing boundaries. + +The reported wall time is the mean time per original workload within each block, +not the total block duration. Raw inner timings and aggregate counters are retained +in the artifacts; phase diagnostics and call counts are normalized per repetition. +Repetition and in-process warmup counts must match across all base/candidate pairs. +Retained single-pass reports remain readable. Repetitions do not increase the number +of independent paired comparisons, and the 1 ms floor remains per original workload. Workers have six minutes each. CI allows 90 minutes inside a 100-minute step and 160-minute job; local runs receive 105 minutes because they build both revisions. @@ -50,8 +66,18 @@ is published. A failed aggregate build can still publish usable profiler artifac Exact-head reports may finalize after merge; stale heads are ignored. Missing, malformed, canceled, incomplete, or invalid data remains unavailable. -The report highlights consistent slowdowns and improvements using the same 20% -median change, 1 ms absolute change, and 80% pair-agreement requirements. +The report highlights consistent slowdowns and improvements in two advisory tiers: + +- Tentative small impact: more than 10% median paired movement. +- Large impact: more than 20% median paired movement. + +Both tiers require at least 1 ms between median per-workload runtimes and at least +80% of pairs exceeding that tier's relative threshold in the same direction. +Large impact is evaluated first. A median above 20% with agreement only at 10% +can therefore remain tentative small impact. Inconsistent movement produces no +headline, badge, or signal fingerprint; its percentage and agreement remain in +the expanded timing table as no signal. Missing environments remain unavailable +and a clean partial assessment is labeled incomplete. The publisher waits up to 220 minutes inside a 230-minute workflow. The first main comparison after introduction may be incomplete because its parent lacks this diff --git a/eng/profiler_benchmarks/controller.py b/eng/profiler_benchmarks/controller.py index 143164e21..7b0f6165d 100644 --- a/eng/profiler_benchmarks/controller.py +++ b/eng/profiler_benchmarks/controller.py @@ -11,6 +11,7 @@ import platform import re import signal +import statistics import subprocess import sys import tarfile @@ -145,6 +146,49 @@ def load_suite(): return core, workloads +def measure_scenario(profiler, name): + repetitions = workloads.MEASUREMENT_REPEATS.get(name, 1) + warmups = 0 if name == "connect" else 1 + results = [] + identity = None + for index in range(warmups + repetitions): + with contextlib.redirect_stdout(io.StringIO()): + result = profiler.run(name)[0] + if result["cpp"] is None or result["py"] is None: + raise RuntimeError(f"Scenario {name} was skipped") + if not result["cpp"]: + raise RuntimeError(f"Scenario {name} has no native samples") + work = result.get("detail", "Connection: 1").split(" (")[0] + if identity is not None and work != identity: + raise RuntimeError(f"Workload changed within scenario {name}") + identity = work + if index >= warmups: + results.append(result) + + times = [result["wall_ms"] for result in results] + output = dict( + wall_ms=statistics.mean(times), + wall_samples_ms=times, + repetitions=repetitions, + warmups=warmups, + work=identity, + ) + for layer in ("cpp", "py"): + stats = {} + for result in results: + for label, counter in result[layer].items(): + if label not in stats: + stats[label] = dict(counter) + else: + total = stats[label] + total["calls"] += counter["calls"] + total["total_us"] += counter["total_us"] + total["min_us"] = min(total["min_us"], counter["min_us"]) + total["max_us"] = max(total["max_us"], counter["max_us"]) + output[layer] = stats + return output + + def worker(args): # Import the chosen driver FIRST, then the SAME workload/controller for both # revisions. Never mix two native extensions into one interpreter. @@ -165,22 +209,14 @@ def worker(args): json.dumps(dict(status="running", active_scenario=name, scenarios=output)), encoding="utf-8", ) - # Keep phase tables out of logs, but never hide which workload stalled. - with contextlib.redirect_stdout(io.StringIO()): - result = profiler.run(name)[0] - if result["cpp"] is None or result["py"] is None: - raise RuntimeError(f"Scenario {name} was skipped") - if not result["cpp"]: - raise RuntimeError(f"Scenario {name} has no native samples") - output[name] = {key: result[key] for key in ("wall_ms", "cpp", "py")} - output[name]["work"] = result.get("detail", "Connection: 1").split(" (")[0] + output[name] = measure_scenario(profiler, name) args.output.write_text( json.dumps( dict(status="running", active_scenario=None, scenarios=output), allow_nan=False ), encoding="utf-8", ) - print(f"Completed scenario: {name} ({result['wall_ms']:.3f} ms)", flush=True) + print(f"Completed scenario: {name} ({output[name]['wall_ms']:.3f} ms)", flush=True) print("Collecting server metadata", flush=True) profiler._ensure_connection() with profiler._conn.cursor() as cursor: diff --git a/eng/profiler_benchmarks/report.py b/eng/profiler_benchmarks/report.py index 07f176f18..f7c57437d 100644 --- a/eng/profiler_benchmarks/report.py +++ b/eng/profiler_benchmarks/report.py @@ -46,7 +46,8 @@ MAX_DIAGNOSTIC_ROWS = 20 MAX_FINGERPRINT_TASKS = 4 MARKER = "" -THRESHOLD = 0.20 +SMALL_THRESHOLD = 0.10 +LARGE_THRESHOLD = 0.20 MIN_DELTA_MS = 1.0 @@ -181,7 +182,21 @@ def _validate(report, build_id=None, head=None, source=None, base=None): number(scenario["wall_ms"]) if scenario["wall_ms"] <= 0: raise ValueError("Zero workload time") - identity = text(scenario["work"]) + repetitions = scenario.get("repetitions", 1) + warmups = scenario.get("warmups", 0) + if type(repetitions) is not int or not 1 <= repetitions <= 100: + raise ValueError("Invalid measurement repetitions") + if type(warmups) is not int or not 0 <= warmups <= 3: + raise ValueError("Invalid scenario warmup count") + if "repetitions" in scenario: + times = scenario["wall_samples_ms"] + if not isinstance(times, list) or len(times) != repetitions: + raise ValueError("Invalid inner timing samples") + if any(number(value) <= 0 for value in times) or not math.isclose( + statistics.mean(times), scenario["wall_ms"], rel_tol=1e-9 + ): + raise ValueError("Inconsistent per-workload time") + identity = (text(scenario["work"]), repetitions, warmups) if name in work and work[name] != identity: raise ValueError(f"Workload changed for {name}") work[name] = identity @@ -276,18 +291,20 @@ def comparisons(report): old = statistics.median(s["wall_ms"] for s in base) new = statistics.median(s["wall_ms"] for s in candidate) ratio = statistics.median(ratios) - # Requiring 80% of paired samples to agree avoids flagging one noisy pass. - agrees = sum(r > 1 + THRESHOLD for r in ratios) >= math.ceil(len(ratios) * 0.8) - improves = sum(r < 1 - THRESHOLD for r in ratios) >= math.ceil(len(ratios) * 0.8) - status = ( - "regression" - if ratio > 1 + THRESHOLD and new - old >= MIN_DELTA_MS and agrees - else ( - "improvement" - if ratio < 1 - THRESHOLD and old - new >= MIN_DELTA_MS and improves - else ("noisy" if ratio > 1 + THRESHOLD and new - old >= MIN_DELTA_MS else "ok") - ) - ) + status, impact = "ok", None + threshold = SMALL_THRESHOLD + direction = 1 if ratio > 1 else -1 + agreement = sum(r > 1 + threshold if direction == 1 else r < 1 - threshold for r in ratios) + for tier, limit in (("large", LARGE_THRESHOLD), ("small", SMALL_THRESHOLD)): + pairs = sum(r > 1 + limit if direction == 1 else r < 1 - limit for r in ratios) + if ( + (ratio > 1 + limit if direction == 1 else ratio < 1 - limit) + and direction * (new - old) >= MIN_DELTA_MS + and pairs >= math.ceil(len(ratios) * 0.8) + ): + status = "regression" if direction == 1 else "improvement" + impact, threshold, agreement = tier, limit, pairs + break phase_deltas = [] changed_counts = [] for layer in ("cpp", "py"): @@ -298,13 +315,23 @@ def comparisons(report): if not all(before) or not all(after): changed_counts.append(f"{label} (added, removed, or intermittent)") continue - before_calls = statistics.median(s["calls"] for s in before) - after_calls = statistics.median(s["calls"] for s in after) + before_calls = statistics.median( + s["calls"] / sample.get("repetitions", 1) for s, sample in zip(before, base) + ) + after_calls = statistics.median( + s["calls"] / sample.get("repetitions", 1) for s, sample in zip(after, candidate) + ) if before_calls != after_calls: changed_counts.append(f"{label} ({before_calls:g} -> {after_calls:g} calls)") delta = ( - statistics.median(s["total_us"] for s in after) - - statistics.median(s["total_us"] for s in before) + statistics.median( + s["total_us"] / sample.get("repetitions", 1) + for s, sample in zip(after, candidate) + ) + - statistics.median( + s["total_us"] / sample.get("repetitions", 1) + for s, sample in zip(before, base) + ) ) / 1000 if delta: phase_deltas.append((delta, label)) @@ -320,6 +347,10 @@ def comparisons(report): candidate_ms=new, change_pct=(ratio - 1) * 100, status=status, + impact=impact, + agreement=agreement, + pairs=len(ratios), + threshold_pct=round(threshold * 100), phases=phases, counts=sorted(changed_counts)[:3], ) @@ -371,12 +402,7 @@ def render(reports, head, build_id, issues=()): for row in rows if row["status"] == "improvement" ] - noisy = [ - (leg, row) - for leg, (_, rows) in completed.items() - for row in rows - if row["status"] == "noisy" - ] + large_regressions = [(leg, row) for leg, row in regressions if row["impact"] == "large"] missing = len(LEGS) - len(completed) highlighted = [ @@ -389,16 +415,13 @@ def render(reports, head, build_id, issues=()): f"{tasks} database task{'s' if tasks != 1 else ''} consistently slowed down across " f"{environments} measured environment{'s' if environments != 1 else ''}." ) - verdict = "⚠️ Performance regression detected" - elif noisy: - tasks = len({row["name"] for _, row in noisy}) - environments = len({leg for leg, _ in noisy}) - opening = ( - f"{tasks} database task{'s' if tasks != 1 else ''} produced inconsistent slowdown " - f"signals across {environments} measured environment" - f"{'s' if environments != 1 else ''}." + verdict = ( + "⚠️ Large performance regression detected" + if large_regressions + else "🔍 Tentative small performance regression" ) - verdict = "🔍 Performance needs review" + if not large_regressions: + opening += " No large-impact slowdowns were detected." elif improvements: tasks = len({row["name"] for _, row in improvements}) environments = len({leg for leg, _ in improvements}) @@ -407,7 +430,11 @@ def render(reports, head, build_id, issues=()): f"{environments} measured environment{'s' if environments != 1 else ''}. " "No consistent slowdowns were detected." ) - verdict = "✅ Performance improved" + verdict = ( + "✅ Large performance improvement detected" + if any(row["impact"] == "large" for _, row in improvements) + else "✅ Tentative small performance improvement" + ) elif not completed: opening = ( "Performance could not be assessed because no environment produced a complete result." @@ -423,10 +450,13 @@ def render(reports, head, build_id, issues=()): f"No consistent slowdowns in the {len(completed)} completed {completed_label}. " f"No result is available for {missing} {missing_label}." ) - verdict = "✅ No regression detected" + verdict = "⚠️ Performance assessment incomplete" - improvement_tasks = len({row["name"] for _, row in improvements}) - regression_tasks = len({row["name"] for _, row in regressions}) + badges = [] + for tier in ("small", "large"): + for entries, label in ((improvements, "IMPROVEMENT"), (regressions, "SLOWDOWN")): + tasks = len({row["name"] for _, row in entries if row["impact"] == tier}) + badges.append(f"{tasks} {tier.upper()} {label}{'S' if tasks != 1 else ''}") lines = [ MARKER, "## PR Performance Report", @@ -435,19 +465,9 @@ def render(reports, head, build_id, issues=()): "", f"**{opening}**", "", - f"{improvement_tasks} IMPROVEMENT" - f"{'S' if improvement_tasks != 1 else ''} " - f"{regression_tasks} SLOWDOWN" - f"{'S' if regression_tasks != 1 else ''} " - f"{len(completed)}/{len(LEGS)} ENVIRONMENTS", + " ".join(badges) + f" {len(completed)}/{len(LEGS)} ENVIRONMENTS", "", ] - if noisy: - noisy_tasks = len({row["name"] for _, row in noisy}) - lines += [ - f"{noisy_tasks} INCONSISTENT SLOWDOWN" f"{'S' if noisy_tasks != 1 else ''}", - "", - ] affected_tasks = [name for name in CASES if any(row["name"] == name for _, row in highlighted)] if highlighted and len(affected_tasks) <= MAX_FINGERPRINT_TASKS: affected_legs = [leg for leg in LEGS if any(item_leg == leg for item_leg, _ in highlighted)] @@ -466,12 +486,10 @@ def render(reports, head, build_id, issues=()): row = by_signal.get((leg, name)) if row is None: cells.append("No signal") - elif row["status"] == "improvement": - cells.append(f"**{abs(row['change_pct']):.1f}% faster**") - elif row["status"] == "regression": - cells.append(f"**{abs(row['change_pct']):.1f}% slower**") else: - cells.append(f"**{abs(row['change_pct']):.1f}% inconsistent**") + direction = "faster" if row["status"] == "improvement" else "slower" + tier = "large" if row["impact"] == "large" else "tentative small" + cells.append(f"**{abs(row['change_pct']):.1f}% {direction} ({tier})**") lines.append(f"| {escape(TASK_NAMES[name])} | " + " | ".join(cells) + " |") lines.append("") if regressions: @@ -479,11 +497,6 @@ def render(reports, head, build_id, issues=()): "The largest recorded phase increases for these tasks are shown below. " "Phase timings are supporting evidence, not root-cause proof." ) - if noisy: - lines.append( - f"{len(noisy)} additional inconsistent slowdown" - f"{'s' if len(noisy) != 1 else ''} also need review." - ) lines.append("") lines += [ @@ -564,19 +577,19 @@ def render(reports, head, build_id, issues=()): lines += [ "", f"### {environment_name(leg)}", - "| Database task | Before | After | Paired change | Result |", - "|---|---:|---:|---:|---|", + "| Database task | Before | After | Paired change | Agreement | Result |", + "|---|---:|---:|---:|---|---|", ] for row in rows: - result = { - "regression": "consistent slowdown", - "improvement": "consistent improvement", - "noisy": "inconsistent slowdown", - "ok": "no signal", - }[row["status"]] + result = "no signal" + if row["impact"]: + tier = "large" if row["impact"] == "large" else "tentative small" + direction = "slowdown" if row["status"] == "regression" else "improvement" + result = f"{tier} {direction}" lines.append( f"| {TASK_NAMES[row['name']]} | {row['base_ms']:.3f} ms | " - f"{row['candidate_ms']:.3f} ms | {row['change_pct']:+.1f}% | {result} |" + f"{row['candidate_ms']:.3f} ms | {row['change_pct']:+.1f}% | " + f"{row['agreement']}/{row['pairs']} beyond {row['threshold_pct']}% | {result} |" ) lines += [ "", @@ -603,17 +616,29 @@ def render(reports, head, build_id, issues=()): lines.append( f"- {environment_name(leg)}: Python {escape(env['python'])}, " f"{escape(env['architecture'])}, SQL {escape(env['sql_version'])}; " - f"{report['samples']} paired comparisons and {report['warmups']} warmup." + f"{report['samples']} paired comparisons and {report['warmups']} discarded warmup pair." ) + repeated = [ + f"{TASK_NAMES[name]}: {scenario['repetitions']} repetitions" + for name, scenario in report["pairs"][0]["base"]["scenarios"].items() + if scenario.get("repetitions", 1) > 1 + ] + if repeated: + lines.append( + " Steady-state tasks warm up once inside each worker; connection opening " + "remains cold. Short-task blocks: " + "; ".join(repeated) + "." + ) lines += [ "", - "A consistent change requires more than 20% median paired movement, at least " - "1 ms between the median runtimes, and at least 80% of pairs exceeding the " - "relative threshold in the same direction. A slowdown without enough pair " - "agreement is reported as inconsistent.", + "Tentative small impact requires more than 10% median paired movement; large impact " + "requires more than 20%. Both require at least 1 ms between the median per-workload " + "runtimes and at least 80% of pairs exceeding that tier's threshold in the same " + "direction. Large impact is evaluated first. Unconfirmed movement remains no signal.", "", "The displayed change is the median of paired before-and-after ratios. It is not " - "recalculated from the two displayed median runtimes.", + "recalculated from the two displayed median runtimes. Repeated blocks use the mean " + "time per original workload; phase timings and call counts are normalized per " + "repetition. Inner repetitions are not additional independent pairs.", ] if issues: lines += ["", "Unavailable or rejected data: " + ", ".join(escape(x) for x in issues)] diff --git a/eng/profiler_benchmarks/workloads.py b/eng/profiler_benchmarks/workloads.py index 9059ab6c9..d5ad5da77 100644 --- a/eng/profiler_benchmarks/workloads.py +++ b/eng/profiler_benchmarks/workloads.py @@ -5,6 +5,17 @@ from profiler import scenarios +# Fixed counts keep both revisions comparable and extend short steady-state tasks +# to roughly 100 ms on the hosted Unix controls. Connection opening stays cold. +MEASUREMENT_REPEATS = { + "select": 100, + "fetchone": 10, + "prepared_qmark": 3, + "prepared_named": 3, + "cte": 20, + "lob_varchar_256k_fetchall": 100, +} + # Preserve the AdventureWorks workloads from the previous CI benchmark. QUERIES = { "join_aggregation": """ diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py index 66ca343b0..d4908d78d 100644 --- a/tests/test_036_profiler_ci.py +++ b/tests/test_036_profiler_ci.py @@ -118,7 +118,7 @@ def test_consistent_slowdown_is_advisory_regression(report): row["status"] == "regression" and row["change_pct"] == pytest.approx(30) for row in rows ) body = reporting.render([report], "c" * 40, 42) - assert "### ⚠️ Performance regression detected" in body + assert "### ⚠️ Large performance regression detected" in body assert f"{len(reporting.CASES)} database tasks consistently slowed down" in body assert "| Unix / SQL Server 2022 | Connection opening |" in body assert "Unavailable: Unix / SQL Server 2025 (incomplete benchmark)." in body @@ -131,7 +131,7 @@ def test_noisy_slowdown_and_submillisecond_change_are_not_regressions(report): for pair in report["pairs"][:2]: pair["candidate"]["scenarios"]["select"]["wall_ms"] = 8 report["pairs"][0]["candidate"]["scenarios"]["connect"]["wall_ms"] = 1000 - assert reporting.comparisons(report)[1]["status"] == "noisy" + assert reporting.comparisons(report)[1]["status"] == "ok" for pair in report["pairs"]: pair["base"]["scenarios"]["insert"]["wall_ms"] = 0.1 pair["candidate"]["scenarios"]["insert"]["wall_ms"] = 0.2 @@ -292,14 +292,155 @@ def clear_slowdowns(report): return report -def test_impact_summary_handles_single_inconsistent_and_complete_clean_results(report): +@pytest.mark.parametrize( + "scale,status,impact", + [ + (1.10, "ok", None), + (1.11, "regression", "small"), + (1.20, "regression", "small"), + (1.21, "regression", "large"), + (0.90, "ok", None), + (0.89, "improvement", "small"), + (0.80, "improvement", "small"), + (0.79, "improvement", "large"), + ], +) +def test_impact_tiers_use_strict_boundaries(report, scale, status, impact): + for pair in report["pairs"]: + pair["base"]["scenarios"]["fetchall"]["wall_ms"] = 100 + pair["candidate"]["scenarios"]["fetchall"]["wall_ms"] = 100 * scale + row = reporting.comparisons(report)[4] + assert (row["status"], row["impact"]) == (status, impact) + + +@pytest.mark.parametrize("scales", [(1.3, 1.3, 1.3, 1.15, 1), (0.7, 0.7, 0.7, 0.85, 1)]) +def test_large_median_with_only_small_tier_agreement_is_small_impact(report, scales): + for pair, scale in zip(report["pairs"], scales): + pair["candidate"]["scenarios"]["fetchall"]["wall_ms"] = 10 * scale + row = reporting.comparisons(report)[4] + assert row["impact"] == "small" + assert row["agreement"] == 4 and row["threshold_pct"] == 10 + + +@pytest.mark.parametrize("scale", [1.15, 0.85]) +def test_small_impact_is_advisory_and_visible(report, scale): + clean = clear_slowdowns(report) + for pair in clean["pairs"]: + pair["candidate"]["scenarios"]["fetchall"]["wall_ms"] *= scale + body = reporting.render([clean], "c" * 40, 42) + direction = "regression" if scale > 1 else "improvement" + assert f"Tentative small performance {direction}" in body + assert "5/5 beyond 10%" in body + assert "tentative small" in body + assert "Advisory result; does not block merging." in body + + +def test_submillisecond_changes_remain_unconfirmed_in_both_tiers(report): + for pair in report["pairs"]: + for name, scale in (("select", 1.15), ("fetchone", 1.3)): + pair["base"]["scenarios"][name]["wall_ms"] = 1 + pair["candidate"]["scenarios"][name]["wall_ms"] = scale + assert all(row["status"] == "ok" for row in reporting.comparisons(report)[1:2]) + assert reporting.comparisons(report)[5]["status"] == "ok" + + +def test_neutral_pr_835_outlier_does_not_produce_an_improvement(report): + clear_slowdowns(report) + for pair, old, new in zip( + report["pairs"], + (35.691, 44.641, 104.950, 36.531, 60.489), + (36.599, 35.560, 36.926, 36.407, 36.442), + ): + pair["base"]["scenarios"]["prepared_named"]["wall_ms"] = old + pair["candidate"]["scenarios"]["prepared_named"]["wall_ms"] = new + row = next(row for row in reporting.comparisons(report) if row["name"] == "prepared_named") + assert row["change_pct"] == pytest.approx(-20.3, abs=0.1) + assert row["status"] == "ok" and row["agreement"] == 3 + body = reporting.render([report], "c" * 40, 42) + assert "Signal fingerprint" not in body + assert "-20.3% | 3/5 beyond 10% | no signal" in body + + +def repeated_report(report): + for pair in report["pairs"]: + for sample in pair.values(): + scenario = sample["scenarios"]["select"] + scenario.update(repetitions=2, warmups=1, wall_samples_ms=[scenario["wall_ms"]] * 2) + scenario["cpp"] = { + "ddbc::query": dict(calls=2, total_us=2000, min_us=1000, max_us=1000) + } + return report + + +@pytest.mark.parametrize( + "field,value", + [ + ("repetitions", True), + ("repetitions", 0), + ("repetitions", 101), + ("warmups", -1), + ("wall_samples_ms", [13]), + ("wall_samples_ms", [13, float("nan")]), + ("wall_samples_ms", [13, -13]), + ("wall_samples_ms", [12, 12]), + ], +) +def test_repeated_samples_are_bounded_and_consistent(report, field, value): + repeated_report(report)["pairs"][0]["candidate"]["scenarios"]["select"][field] = value + with pytest.raises(ValueError): + reporting.validate(report) + + +@pytest.mark.parametrize("field", ["repetitions", "warmups"]) +def test_repetition_contract_must_match_between_revisions(report, field): + repeated_report(report) + scenario = report["pairs"][0]["candidate"]["scenarios"]["select"] + scenario[field] = 3 + if field == "repetitions": + scenario["wall_samples_ms"] = [scenario["wall_ms"]] * 3 + with pytest.raises(ValueError, match="Workload changed"): + reporting.validate(report) + + +def test_phase_diagnostics_are_normalized_per_repetition(report): + repeated_report(report) + for pair in report["pairs"]: + pair["candidate"]["scenarios"]["select"]["cpp"]["ddbc::query"] = dict( + calls=4, total_us=6000, min_us=1500, max_us=1500 + ) + reporting.validate(report) + row = reporting.comparisons(report)[1] + assert row["base_ms"] == 10 and row["candidate_ms"] == 13 + assert row["phases"] == [(2.0, "ddbc::query")] + assert row["counts"] == ["ddbc::query (1 -> 2 calls)"] + assert row["pairs"] == 5 + + +def test_repetition_does_not_inflate_absolute_impact_floor(report): + for pair in report["pairs"]: + for side, wall in (("base", 1), ("candidate", 1.3)): + pair[side]["scenarios"]["select"].update( + wall_ms=wall, + wall_samples_ms=[wall] * 100, + repetitions=100, + warmups=1, + cpp={"ddbc::query": dict(calls=100, total_us=100000, min_us=1000, max_us=1000)}, + ) + reporting.validate(report) + row = reporting.comparisons(report)[1] + assert row["agreement"] == 5 and row["pairs"] == 5 + assert row["status"] == "ok" and row["impact"] is None + + +def test_inconsistent_movement_stays_out_of_summary_and_signals(report): clean = clear_slowdowns(copy.deepcopy(report)) for pair, scale in zip(clean["pairs"], (1.3, 1.3, 1.3, 0.8, 0.8)): pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= scale noisy = reporting.render([clean], "c" * 40, 42) - assert "### 🔍 Performance needs review" in noisy - assert "1 database task produced inconsistent slowdown signals" in noisy - assert "1 INCONSISTENT SLOWDOWN" in noisy + assert "### ⚠️ Performance assessment incomplete" in noisy + assert "INCONSISTENT" not in noisy + assert "Signal fingerprint" not in noisy + assert "+30.0% | 3/5 beyond 10% | no signal" in noisy complete = [set_leg(clear_slowdowns(copy.deepcopy(report)), leg) for leg in reporting.LEGS] clean_body = reporting.render(complete, "c" * 40, 42) @@ -313,7 +454,7 @@ def test_impact_summary_handles_single_regression_partial_and_no_results(report) for pair in single["pairs"]: pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= 1.3 body = reporting.render([single], "c" * 40, 42) - assert "### ⚠️ Performance regression detected" in body + assert "### ⚠️ Large performance regression detected" in body assert "**1 database task consistently slowed down across 1 measured environment.**" in body assert "Performance diagnostics" in body assert "All database tasks and timings" in body @@ -349,19 +490,21 @@ def test_impact_summary_reports_consistent_improvements(report): assert rows[4]["status"] == "improvement" assert rows[4]["phases"] == [(-0.5, "ddbc::query")] body = reporting.render(reports, "c" * 40, 42) - assert "### ✅ Performance improved" in body + assert "### ✅ Large performance improvement detected" in body assert ( "**2 database tasks consistently improved across 2 measured environments. " "No consistent slowdowns were detected.**" ) in body - assert "2 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS" in body - assert "| Fetch-all queries | **30.0% faster** | **28.0% faster** |" in body + assert "2 LARGE IMPROVEMENTS 0 LARGE SLOWDOWNS" in body + assert "2/2 ENVIRONMENTS" in body + assert "| Fetch-all queries | **30.0% faster (large)** | **28.0% faster (large)** |" in body assert ( - "| Insertion with explicit input sizes | **40.0% faster** | " "**39.0% faster** |" + "| Insertion with explicit input sizes | **40.0% faster (large)** | " + "**39.0% faster (large)** |" ) in body assert "Spread" not in body assert "Measured timings" in body - assert "| Fetch-all queries |" in body and "| consistent improvement |" in body + assert "| Fetch-all queries |" in body and "| large improvement |" in body assert "ddbc::query -0.500 ms" in body @@ -371,20 +514,21 @@ def test_regression_headline_keeps_precedence_over_improvement(report): pair["candidate"]["scenarios"]["fetchall"]["wall_ms"] *= 0.7 pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= 1.3 body = reporting.render([mixed], "c" * 40, 42) - assert "### ⚠️ Performance regression detected" in body - assert "1 IMPROVEMENT 1 SLOWDOWN" in body + assert "### ⚠️ Large performance regression detected" in body + assert "1 LARGE IMPROVEMENT 1 LARGE SLOWDOWN" in body -def test_inconsistent_slowdown_keeps_precedence_over_improvement(report): +def test_inconsistent_slowdown_does_not_override_consistent_improvement(report): mixed = clear_slowdowns(copy.deepcopy(report)) for pair in mixed["pairs"]: pair["candidate"]["scenarios"]["fetchall"]["wall_ms"] *= 0.7 for pair, scale in zip(mixed["pairs"], (1.3, 1.3, 1.3, 0.8, 0.8)): pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= scale body = reporting.render([mixed], "c" * 40, 42) - assert "### 🔍 Performance needs review" in body - assert "1 IMPROVEMENT 0 SLOWDOWNS" in body - assert "1 INCONSISTENT SLOWDOWN" in body + assert "### ✅ Large performance improvement detected" in body + assert "1 LARGE IMPROVEMENT 0 LARGE SLOWDOWNS" in body + assert "INCONSISTENT" not in body + assert "+30.0% | 3/5 beyond 10% | no signal" in body @pytest.mark.parametrize( @@ -868,10 +1012,80 @@ def test_legacy_insert_workload_executes_both_variants(input_sizes): context.disable.assert_called_once() +def test_short_measurements_warm_inside_worker_and_retain_raw_repetitions(monkeypatch): + monkeypatch.setitem(benchmark_workloads.MEASUREMENT_REPEATS, "select", 2) + + def sample(wall, calls, total): + return dict( + wall_ms=wall, + cpp={"ddbc::query": dict(calls=calls, total_us=total, min_us=100, max_us=200)}, + py={}, + detail="Rows: 100", + ) + + profiler = MagicMock() + profiler.run.side_effect = [ + [sample(1000, 1000, 100000)], + [sample(4, 2, 400)], + [sample(8, 3, 600)], + ] + result = controller.measure_scenario(profiler, "select") + assert profiler.run.call_count == 3 + assert result["warmups"] == 1 and result["repetitions"] == 2 + assert result["wall_samples_ms"] == [4, 8] and result["wall_ms"] == 6 + assert result["cpp"]["ddbc::query"] == dict(calls=5, total_us=1000, min_us=100, max_us=200) + assert result["work"] == "Rows: 100" + + +def test_connection_measurement_remains_cold_and_single_pass(): + profiler = MagicMock() + profiler.run.return_value = [dict(wall_ms=10, cpp={"ddbc::connect": {}}, py={})] + result = controller.measure_scenario(profiler, "connect") + profiler.run.assert_called_once_with("connect") + assert result["warmups"] == 0 and result["repetitions"] == 1 + assert result["work"] == "Connection: 1" + + +def test_long_steady_state_task_warms_once_without_repeating_measurement(): + profiler = MagicMock() + profiler.run.side_effect = [ + [dict(wall_ms=9000, cpp={"ddbc::fetch": {}}, py={}, detail="Rows: 1200000")], + [dict(wall_ms=3000, cpp={"ddbc::fetch": {}}, py={}, detail="Rows: 1200000")], + ] + result = controller.measure_scenario(profiler, "fetch_1_2m") + assert profiler.run.call_count == 2 + assert result["warmups"] == 1 and result["repetitions"] == 1 + assert result["wall_ms"] == 3000 and result["wall_samples_ms"] == [3000] + + +def test_in_process_warmup_failure_is_not_hidden(): + profiler = MagicMock() + profiler.run.side_effect = RuntimeError("warmup failed") + with pytest.raises(RuntimeError, match="warmup failed"): + controller.measure_scenario(profiler, "select") + profiler.run.assert_called_once() + + +def test_repeated_workload_cannot_change_after_warmup(monkeypatch): + monkeypatch.setitem(benchmark_workloads.MEASUREMENT_REPEATS, "select", 2) + profiler = MagicMock() + profiler.run.side_effect = [ + [dict(wall_ms=1, cpp={"ddbc::query": {}}, py={}, detail=f"Rows: {rows}")] + for rows in (100, 200) + ] + with pytest.raises(RuntimeError, match="Workload changed"): + controller.measure_scenario(profiler, "select") + + @pytest.mark.parametrize("fail", [False, True]) def test_worker_checkpoints_completed_and_active_scenarios(tmp_path, monkeypatch, capsys, fail): output = tmp_path / "base-0.json" - result = dict(wall_ms=10.0, cpp={"ddbc::run": {}}, py={}, detail="Rows: 10") + result = dict( + wall_ms=10.0, + cpp={"ddbc::run": dict(calls=1, total_us=1000, min_us=1000, max_us=1000)}, + py={}, + detail="Rows: 10", + ) profiler = MagicMock() profiler.__enter__.return_value = profiler profiler._conn.cursor.return_value.__enter__.return_value.fetchone.return_value = ("16.0",)