From 9f438e6cf62d925a09de0e3436f098f035579337 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 07:53:35 -0700 Subject: [PATCH 1/8] test: tighten post-fix macOS performance budgets (Fixes #507) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 10 ++++++---- scripts/quality_snapshot.py | 6 +++--- scripts/tests/test_quality_snapshot.py | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 79461ffa..dac9c76d 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -15,14 +15,16 @@ A metric blocks when it exceeds both its absolute and relative budget: | Metric | Linux | Windows | macOS | | --- | ---: | ---: | ---: | | Server startup P50 | 5 ms / 100% | 10 ms / 50% | 100 ms / 50% | -| Server startup P95 | 50 ms / 200% | 50 ms / 100% | 10,000 ms / 100% | +| Server startup P95 | 50 ms / 200% | 50 ms / 100% | 750 ms / 100% | | Full refresh P50 | 25 ms / 30% | 50 ms / 30% | 100 ms / 50% | -| Full refresh P95 | 1,000 ms / 100% | 5,000 ms / 100% | 5,000 ms / 25% | +| Full refresh P95 | 1,000 ms / 100% | 5,000 ms / 100% | 1,000 ms / 50% | | Time to first environment P50 | 20 ms / 100% | 25 ms / 50% | 150 ms / 50% | -| Time to first environment P95 | 250 ms / 100% | 500 ms / 100% | 10,000 ms / 100% | +| Time to first environment P95 | 250 ms / 100% | 500 ms / 100% | 750 ms / 100% | Each cell is `absolute / relative`. The budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. +The macOS P95 budgets were recalibrated after #504 using three unchanged-content pull-request runs and the exact merged baseline. Their absolute headroom is four to six times the observed post-fix run-to-run range. + The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Tail metrics remain mandatory; a healthy median does not excuse a degraded P95. ## Coverage gate @@ -50,4 +52,4 @@ Phase and locator telemetry is collected in separate, untimed refreshes so diagn ## Known investigations -The macOS cold-refresh tail is tracked by issue #504. Phase and locator distributions plus privacy-safe interpreter timeout counts verify that the tail does not recur. +The macOS cold-refresh tail fixed by issue #504 remains guarded by phase and locator distributions plus privacy-safe interpreter timeout counts. diff --git a/scripts/quality_snapshot.py b/scripts/quality_snapshot.py index 250eaf49..6e77a5c3 100644 --- a/scripts/quality_snapshot.py +++ b/scripts/quality_snapshot.py @@ -80,11 +80,11 @@ def regressed(self) -> bool: ), 'macos': ( RegressionBudget(100, 50), - RegressionBudget(10_000, 100), + RegressionBudget(750, 100), RegressionBudget(100, 50), - RegressionBudget(5_000, 25), + RegressionBudget(1_000, 50), RegressionBudget(150, 50), - RegressionBudget(10_000, 100), + RegressionBudget(750, 100), ), } COVERAGE_BUDGET_PERCENTAGE_POINTS = 0.01 diff --git a/scripts/tests/test_quality_snapshot.py b/scripts/tests/test_quality_snapshot.py index 6a5cb6a5..97ad8562 100644 --- a/scripts/tests/test_quality_snapshot.py +++ b/scripts/tests/test_quality_snapshot.py @@ -64,6 +64,27 @@ def test_p95_regression_fails_even_when_p50_is_unchanged(self): _, failures = compare_performance(current, performance_snapshot(refresh_p95=500), 'Windows') self.assertTrue(any('Full refresh P95' in failure for failure in failures)) + def test_post_fix_macos_tail_variance_passes(self): + baseline = performance_snapshot(startup_p95=621, refresh_p95=1_343, first_p95=649) + current = performance_snapshot(startup_p95=691, refresh_p95=1_435, first_p95=745) + + _, failures = compare_performance(current, baseline, 'macOS') + + self.assertEqual(failures, []) + + def test_tightened_macos_tail_budgets_reject_multi_second_regressions(self): + baseline = performance_snapshot(startup_p95=621, refresh_p95=1_343, first_p95=649) + current = performance_snapshot(startup_p95=1_500, refresh_p95=2_500, first_p95=1_500) + + _, failures = compare_performance(current, baseline, 'macOS') + + for label in ( + 'Server startup P95', + 'Full refresh P95', + 'Time to first environment P95', + ): + self.assertTrue(any(label in failure for failure in failures)) + def test_noise_inside_absolute_budget_passes(self): current = performance_snapshot(refresh_p50=140) _, failures = compare_performance(current, performance_snapshot(refresh_p50=100), 'Windows') From 0ef48b5c10b915c1ea9304896bc1113acdceef76 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 07:59:00 -0700 Subject: [PATCH 2/8] docs: clarify macOS budget calibration source (PR #508) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index dac9c76d..7dea7109 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -23,7 +23,7 @@ A metric blocks when it exceeds both its absolute and relative budget: Each cell is `absolute / relative`. The budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. -The macOS P95 budgets were recalibrated after #504 using three unchanged-content pull-request runs and the exact merged baseline. Their absolute headroom is four to six times the observed post-fix run-to-run range. +The macOS P95 budgets were recalibrated after PR #506 (tracking issue #504) using three unchanged-content pull-request runs and the exact merged baseline at `f0c62d9`. Their absolute headroom is four to six times the observed post-fix run-to-run range. The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Tail metrics remain mandatory; a healthy median does not excuse a degraded P95. @@ -52,4 +52,4 @@ Phase and locator telemetry is collected in separate, untimed refreshes so diagn ## Known investigations -The macOS cold-refresh tail fixed by issue #504 remains guarded by phase and locator distributions plus privacy-safe interpreter timeout counts. +The macOS cold-refresh tail fixed by PR #506 (tracked in issue #504) remains guarded by phase and locator distributions plus privacy-safe interpreter timeout counts. From e49ed7673de52b62f8a529bb52c576143bcb5c1a Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 08:05:32 -0700 Subject: [PATCH 3/8] docs: distinguish macOS P95 calibration data (PR #508) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 7dea7109..2b1c0e5e 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -21,7 +21,7 @@ A metric blocks when it exceeds both its absolute and relative budget: | Time to first environment P50 | 20 ms / 100% | 25 ms / 50% | 150 ms / 50% | | Time to first environment P95 | 250 ms / 100% | 500 ms / 100% | 750 ms / 100% | -Each cell is `absolute / relative`. The budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. +Each cell is `absolute / relative`. The Linux and Windows budgets plus the macOS P50 budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. The macOS P95 budgets were recalibrated after PR #506 (tracking issue #504) using three unchanged-content pull-request runs and the exact merged baseline at `f0c62d9`. Their absolute headroom is four to six times the observed post-fix run-to-run range. From 412b102c99f3ec9ea590086d1c811a9da5d833a4 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 08:09:17 -0700 Subject: [PATCH 4/8] docs: identify macOS budget tracking issue (PR #508) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 2b1c0e5e..0d20456e 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -23,7 +23,7 @@ A metric blocks when it exceeds both its absolute and relative budget: Each cell is `absolute / relative`. The Linux and Windows budgets plus the macOS P50 budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. -The macOS P95 budgets were recalibrated after PR #506 (tracking issue #504) using three unchanged-content pull-request runs and the exact merged baseline at `f0c62d9`. Their absolute headroom is four to six times the observed post-fix run-to-run range. +The macOS P95 budget recalibration is tracked by issue #507 and follows PR #506's fix for issue #504. It uses three unchanged-content pull-request runs and the exact merged baseline at `f0c62d9`; the resulting absolute headroom is four to six times the observed post-fix run-to-run range. The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Tail metrics remain mandatory; a healthy median does not excuse a degraded P95. From b96da551626e75dc802d0c29f31bbbce33aba80e Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 08:54:57 -0700 Subject: [PATCH 5/8] test: separate cold and warm performance samples (#509) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/tests/e2e_performance.rs | 222 +++++++++++++++++++++------- 1 file changed, 167 insertions(+), 55 deletions(-) diff --git a/crates/pet/tests/e2e_performance.rs b/crates/pet/tests/e2e_performance.rs index 7a6cf453..cd286980 100644 --- a/crates/pet/tests/e2e_performance.rs +++ b/crates/pet/tests/e2e_performance.rs @@ -28,6 +28,7 @@ static REQUEST_ID: AtomicU32 = AtomicU32::new(1); /// Number of iterations for statistical tests const STAT_ITERATIONS: usize = 10; +const PERFORMANCE_METRICS_SCHEMA_VERSION: u8 = 2; const STDERR_TAIL_LINES: usize = 100; /// Statistical metrics with percentile calculations @@ -552,6 +553,40 @@ fn get_test_cache_dir() -> PathBuf { .join(format!("cache-{}", std::process::id())) } +fn benchmark_iteration_cache_dir(cache_root: &Path, workload: &str, iteration: usize) -> PathBuf { + cache_root + .join(workload) + .join(format!("iteration-{}", iteration + 1)) +} + +fn reset_cache_dir(cache_dir: &Path) { + match std::fs::remove_dir_all(cache_dir) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => panic!("Failed to remove cache directory {cache_dir:?}: {error}"), + } + std::fs::create_dir_all(cache_dir) + .unwrap_or_else(|error| panic!("Failed to create cache directory {cache_dir:?}: {error}")); +} + +fn assert_stable_inventory( + expected: &mut Option<(usize, usize)>, + actual: (usize, usize), + workload: &str, + iteration: usize, +) { + if let Some(expected) = expected { + assert_eq!( + actual, + *expected, + "{workload} inventory changed at iteration {}", + iteration + 1 + ); + } else { + *expected = Some(actual); + } +} + /// Get workspace directory (current project root) fn get_workspace_dir() -> PathBuf { env::var("GITHUB_WORKSPACE") @@ -763,20 +798,22 @@ fn collect_refresh_diagnostics( phase_stats: &mut BTreeMap, locator_stats: &mut BTreeMap, probe_timeout_counts: &mut BTreeMap, + expected_inventory: (usize, usize), ) { - let diagnostic_cache_dir = cache_dir.join("refresh-progress"); - let _ = std::fs::remove_dir_all(&diagnostic_cache_dir); - std::fs::create_dir_all(&diagnostic_cache_dir) - .expect("Failed to create refresh diagnostic cache dir"); + let diagnostic_cache_root = cache_dir.join("refresh-progress"); + reset_cache_dir(&diagnostic_cache_root); - println!("\nCollecting untimed refresh diagnostics..."); + println!("\nCollecting untimed cold-refresh diagnostics..."); for iteration in 0..STAT_ITERATIONS { + let diagnostic_cache_dir = + benchmark_iteration_cache_dir(&diagnostic_cache_root, "cold", iteration); + reset_cache_dir(&diagnostic_cache_dir); let mut client = PetClient::spawn_with_refresh_progress().expect("Failed to spawn diagnostic server"); client .configure(json!({ "workspaceDirectories": [workspace_dir], - "cacheDirectory": diagnostic_cache_dir + "cacheDirectory": diagnostic_cache_dir, })) .expect("Failed to configure diagnostic server"); let (result, _) = client @@ -784,12 +821,19 @@ fn collect_refresh_diagnostics( .expect("Failed to run diagnostic refresh"); collect_refresh_progress(&client.get_refresh_progress(), phase_stats, locator_stats); + let inventory = (client.get_environments().len(), client.get_managers().len()); + assert_eq!( + inventory, + expected_inventory, + "Cold diagnostic inventory changed at iteration {}", + iteration + 1, + ); record_interpreter_probe_timeouts(&client, probe_timeout_counts); println!( - " Diagnostic iteration {}: refresh={}ms, envs={}", + " Cold diagnostic iteration {}: refresh={}ms, envs={}", iteration + 1, result.duration, - client.get_environments().len() + inventory.0, ); } } @@ -853,6 +897,18 @@ fn refresh_progress_aggregation_separates_phases_and_locators() { assert_eq!(locators["Conda"].samples, vec![20]); } +#[test] +fn benchmark_cache_directories_are_isolated_by_workload_and_iteration() { + let root = Path::new("benchmark-cache"); + let first_cold = benchmark_iteration_cache_dir(root, "cold", 0); + let second_cold = benchmark_iteration_cache_dir(root, "cold", 1); + let first_warm = benchmark_iteration_cache_dir(root, "warm", 0); + + assert_eq!(first_cold, root.join("cold").join("iteration-1")); + assert_ne!(first_cold, second_cold); + assert_ne!(first_cold, first_warm); +} + // ============================================================================ // Performance Tests // ============================================================================ @@ -1340,73 +1396,124 @@ fn test_refresh_warm_vs_cold_cache() { #[allow(dead_code)] fn test_performance_summary() { let mut startup_stats = StatisticalMetrics::new(); - let mut refresh_stats = StatisticalMetrics::new(); - let mut time_to_first_env_stats = StatisticalMetrics::new(); + let mut cold_refresh_stats = StatisticalMetrics::new(); + let mut warm_refresh_stats = StatisticalMetrics::new(); + let mut cold_time_to_first_env_stats = StatisticalMetrics::new(); + let mut warm_time_to_first_env_stats = StatisticalMetrics::new(); let mut phase_stats = BTreeMap::new(); let mut locator_stats = BTreeMap::new(); let mut probe_timeout_counts: BTreeMap = BTreeMap::new(); let mut expected_inventory = None; - let cache_dir = get_test_cache_dir(); - let _ = std::fs::remove_dir_all(&cache_dir); - std::fs::create_dir_all(&cache_dir).expect("Failed to create cache dir"); - + let cache_root = get_test_cache_dir(); + reset_cache_dir(&cache_root); let workspace_dir = get_workspace_dir(); println!("\n========================================"); - println!(" PERFORMANCE SUMMARY ({} iterations)", STAT_ITERATIONS); + println!( + " COLD/WARM PERFORMANCE SUMMARY ({} pairs)", + STAT_ITERATIONS + ); println!("========================================\n"); - for i in 0..STAT_ITERATIONS { - // Measure server startup (fresh server each iteration) - let spawn_start = Instant::now(); - let mut client = PetClient::spawn().expect("Failed to spawn server"); - - let config = json!({ - "workspaceDirectories": [workspace_dir.clone()], - "cacheDirectory": cache_dir.clone() - }); + for iteration in 0..STAT_ITERATIONS { + let iteration_cache = benchmark_iteration_cache_dir(&cache_root, "measured", iteration); + reset_cache_dir(&iteration_cache); - client.configure(config).expect("Failed to configure"); + let spawn_start = Instant::now(); + let mut cold_client = PetClient::spawn().expect("Failed to spawn cold server"); + cold_client + .configure(json!({ + "workspaceDirectories": [workspace_dir.clone()], + "cacheDirectory": iteration_cache.clone(), + })) + .expect("Failed to configure cold server"); let startup_time = spawn_start.elapsed().as_millis(); startup_stats.add(startup_time); - // Measure full refresh - let (result, _) = client.refresh(None).expect("Failed to refresh"); - refresh_stats.add(result.duration); - - let inventory = (client.get_environments().len(), client.get_managers().len()); - if let Some(expected) = expected_inventory { - assert_eq!( - inventory, expected, - "Environment and manager inventory changed after iteration 1" - ); - } else { - expected_inventory = Some(inventory); + let (cold_result, _) = cold_client + .refresh(None) + .expect("Failed to run cold refresh"); + cold_refresh_stats.add(cold_result.duration); + let cold_inventory = ( + cold_client.get_environments().len(), + cold_client.get_managers().len(), + ); + assert_stable_inventory( + &mut expected_inventory, + cold_inventory, + "Cold refresh", + iteration, + ); + if let Some(ttfe) = cold_client.time_to_first_env() { + cold_time_to_first_env_stats.add(ttfe.as_millis()); } + record_interpreter_probe_timeouts(&cold_client, &mut probe_timeout_counts); + + println!( + " Cold iteration {}: startup={}ms, refresh={}ms, envs={}", + iteration + 1, + startup_time, + cold_result.duration, + cold_inventory.0, + ); + drop(cold_client); - if let Some(ttfe) = client.time_to_first_env() { - time_to_first_env_stats.add(ttfe.as_millis()); + let mut warm_client = PetClient::spawn().expect("Failed to spawn warm server"); + warm_client + .configure(json!({ + "workspaceDirectories": [workspace_dir.clone()], + "cacheDirectory": iteration_cache, + })) + .expect("Failed to configure warm server"); + let (warm_result, _) = warm_client + .refresh(None) + .expect("Failed to run warm refresh"); + warm_refresh_stats.add(warm_result.duration); + let warm_inventory = ( + warm_client.get_environments().len(), + warm_client.get_managers().len(), + ); + assert_stable_inventory( + &mut expected_inventory, + warm_inventory, + "Warm refresh", + iteration, + ); + if let Some(ttfe) = warm_client.time_to_first_env() { + warm_time_to_first_env_stats.add(ttfe.as_millis()); } - record_interpreter_probe_timeouts(&client, &mut probe_timeout_counts); + record_interpreter_probe_timeouts(&warm_client, &mut probe_timeout_counts); println!( - " Iteration {}: startup={}ms, refresh={}ms, envs={}", - i + 1, - startup_time, - result.duration, - inventory.0 + " Warm iteration {}: refresh={}ms, envs={}", + iteration + 1, + warm_result.duration, + warm_inventory.0, ); } let (env_count, manager_count) = expected_inventory.expect("Performance summary must run at least one iteration"); + for (label, count) in [ + ("startup", startup_stats.count()), + ("cold refresh", cold_refresh_stats.count()), + ("warm refresh", warm_refresh_stats.count()), + ("cold time-to-first", cold_time_to_first_env_stats.count()), + ("warm time-to-first", warm_time_to_first_env_stats.count()), + ] { + assert_eq!( + count, STAT_ITERATIONS, + "Expected one {label} sample per benchmark pair" + ); + } collect_refresh_diagnostics( &workspace_dir, - &cache_dir, + &cache_root, &mut phase_stats, &mut locator_stats, &mut probe_timeout_counts, + (env_count, manager_count), ); for phase in ["locators", "path", "globalVirtualEnvs", "workspaces"] { @@ -1429,10 +1536,10 @@ fn test_performance_summary() { println!(" STATISTICS "); println!("----------------------------------------"); startup_stats.print_summary("Server startup"); - refresh_stats.print_summary("Full refresh"); - if time_to_first_env_stats.count() > 0 { - time_to_first_env_stats.print_summary("Time to first env"); - } + cold_refresh_stats.print_summary("Cold full refresh"); + warm_refresh_stats.print_summary("Warm full refresh"); + cold_time_to_first_env_stats.print_summary("Cold time to first env"); + warm_time_to_first_env_stats.print_summary("Warm time to first env"); for (phase, metrics) in &phase_stats { metrics.print_summary(&format!("Phase {phase}")); } @@ -1447,17 +1554,22 @@ fn test_performance_summary() { let locator_json = statistics_json(&locator_stats); // Output as JSON for CI parsing - // Includes both P50 values at top level (for backwards compatibility) and full stats + // Existing top-level refresh fields remain warm-cache values for schema compatibility. let json_output = serde_json::to_string_pretty(&json!({ + "metrics_schema_version": PERFORMANCE_METRICS_SCHEMA_VERSION, "server_startup_ms": startup_stats.p50().unwrap_or(0), - "full_refresh_ms": refresh_stats.p50().unwrap_or(0), - "time_to_first_env_ms": time_to_first_env_stats.p50(), + "full_refresh_ms": warm_refresh_stats.p50().unwrap_or(0), + "cold_refresh_ms": cold_refresh_stats.p50().unwrap_or(0), + "time_to_first_env_ms": warm_time_to_first_env_stats.p50(), + "cold_time_to_first_env_ms": cold_time_to_first_env_stats.p50(), "environments_count": env_count, "managers_count": manager_count, "stats": { "server_startup": startup_stats.to_json(), - "full_refresh": refresh_stats.to_json(), - "time_to_first_env": time_to_first_env_stats.to_json() + "full_refresh": warm_refresh_stats.to_json(), + "cold_refresh": cold_refresh_stats.to_json(), + "time_to_first_env": warm_time_to_first_env_stats.to_json(), + "cold_time_to_first_env": cold_time_to_first_env_stats.to_json(), }, "phases": phase_json, "locators": locator_json, From 1f1990812ad948da91a82a66218bf4a72ce90526 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 09:19:11 -0700 Subject: [PATCH 6/8] test: enforce robust cold refresh performance gates (#509) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/QUALITY_SNAPSHOTS.md | 10 +- scripts/quality_snapshot.py | 148 ++++++++++++++++++++++--- scripts/tests/test_quality_snapshot.py | 98 +++++++++++++++- 3 files changed, 237 insertions(+), 19 deletions(-) diff --git a/docs/QUALITY_SNAPSHOTS.md b/docs/QUALITY_SNAPSHOTS.md index 0d20456e..ffadd3ec 100644 --- a/docs/QUALITY_SNAPSHOTS.md +++ b/docs/QUALITY_SNAPSHOTS.md @@ -4,7 +4,7 @@ PET uses pull-request snapshots to prevent performance and coverage drift. Each ## Performance gate -The performance workflow runs 10 end-to-end JSON-RPC iterations on Linux, Windows, and macOS. A comparison is valid only when: +The performance workflow runs 10 paired cache-cold/cache-warm JSON-RPC iterations on Linux, Windows, and macOS, plus 10 untimed cache-cold diagnostic iterations. A comparison is valid only when: - current and baseline metrics contain at least five samples for every required distribution; - environment and manager counts match exactly; and @@ -20,12 +20,15 @@ A metric blocks when it exceeds both its absolute and relative budget: | Full refresh P95 | 1,000 ms / 100% | 5,000 ms / 100% | 1,000 ms / 50% | | Time to first environment P50 | 20 ms / 100% | 25 ms / 50% | 150 ms / 50% | | Time to first environment P95 | 250 ms / 100% | 500 ms / 100% | 750 ms / 100% | +| Cold refresh P50 | 100 ms / 50% | 150 ms / 50% | 250 ms / 50% | Each cell is `absolute / relative`. The Linux and Windows budgets plus the macOS P50 budgets reflect observed GitHub-hosted runner variance from 11 consecutive main-branch baselines. Tighten them when a noisy path is fixed rather than normalizing a known regression into the baseline. The macOS P95 budget recalibration is tracked by issue #507 and follows PR #506's fix for issue #504. It uses three unchanged-content pull-request runs and the exact merged baseline at `f0c62d9`; the resulting absolute headroom is four to six times the observed post-fix run-to-run range. -The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Tail metrics remain mandatory; a healthy median does not excuse a degraded P95. +Schema v2 records `full_refresh` and `time_to_first_env` from the warm member of each pair and adds cold refresh/time-to-first distributions. While the exact base still uses schema v1, cold P50 is checked against explicit absolute ceilings of 500ms on Linux, 750ms on Windows, and 1,000ms on macOS. Once both snapshots use schema v2, the table's dual budgets apply. + +The dual budget avoids failing on tiny percentage changes while still blocking material latency regressions. Warm tail metrics remain mandatory; cold P95 remains diagnostic because a single host event can dominate it, while cold P50 blocks delays that affect the independent cold iterations consistently. ## Coverage gate @@ -48,8 +51,9 @@ cargo test --release --features ci-perf --test e2e_performance test_performance_ ``` The E2E client keeps one buffered stdout reader for the process lifetime and continuously drains a bounded stderr tail so protocol read-ahead and pipe backpressure cannot distort measurements. -Phase and locator telemetry is collected in separate, untimed refreshes so diagnostic processing cannot backpressure the timed JSON-RPC refreshes. +Each cold/warm pair uses a unique cache directory and a fresh PET process for each member. Phase and locator telemetry is collected across separate, independently cold untimed refreshes so diagnostic processing cannot backpressure the timed JSON-RPC refreshes. ## Known investigations The macOS cold-refresh tail fixed by PR #506 (tracked in issue #504) remains guarded by phase and locator distributions plus privacy-safe interpreter timeout counts. +The cold/warm sampling design is tracked by issue #509. diff --git a/scripts/quality_snapshot.py b/scripts/quality_snapshot.py index 6e77a5c3..8d613558 100644 --- a/scripts/quality_snapshot.py +++ b/scripts/quality_snapshot.py @@ -53,6 +53,24 @@ def regressed(self) -> bool: return self.delta > self.budget.absolute_ms and self.percent_change > self.budget.relative_percent +@dataclass(frozen=True) +class AbsoluteLimitComparison: + label: str + current: float + limit: float + + @property + def delta(self) -> float: + return self.current - self.limit + + @property + def regressed(self) -> bool: + return self.current > self.limit + + +PerformanceComparison = MetricComparison | AbsoluteLimitComparison + + PERFORMANCE_METRICS = ( MetricSpec('Server startup P50', 'server_startup', 'p50'), MetricSpec('Server startup P95', 'server_startup', 'p95'), @@ -87,6 +105,19 @@ def regressed(self) -> bool: RegressionBudget(750, 100), ), } +PERFORMANCE_METRICS_SCHEMA_VERSION = 2 +COLD_REFRESH_SPEC = MetricSpec('Cold refresh P50', 'cold_refresh', 'p50') +COLD_DIAGNOSTIC_SPECS = ( + MetricSpec('Cold refresh P95', 'cold_refresh', 'p95'), + MetricSpec('Cold time to first environment P50', 'cold_time_to_first_env', 'p50'), + MetricSpec('Cold time to first environment P95', 'cold_time_to_first_env', 'p95'), +) +COLD_REFRESH_BUDGETS = { + 'linux': RegressionBudget(100, 50), + 'windows': RegressionBudget(150, 50), + 'macos': RegressionBudget(250, 50), +} +COLD_REFRESH_LEGACY_LIMIT_MS = {'linux': 500, 'windows': 750, 'macos': 1_000} COVERAGE_BUDGET_PERCENTAGE_POINTS = 0.01 @@ -151,9 +182,54 @@ def performance_value(snapshot: dict[str, Any], spec: MetricSpec, source: str) - return require_number(group.get(spec.percentile), f'{source}.stats.{spec.group}.{spec.percentile}') +def performance_schema_version(snapshot: dict[str, Any], source: str) -> int: + version = require_integer( + snapshot.get('metrics_schema_version', 1), + f'{source}.metrics_schema_version', + minimum=1, + ) + if version > PERFORMANCE_METRICS_SCHEMA_VERSION: + raise SnapshotError( + f'{source}.metrics_schema_version {version} is newer than supported version ' + f'{PERFORMANCE_METRICS_SCHEMA_VERSION}' + ) + return version + + +def cold_refresh_budget(platform: str) -> RegressionBudget: + key = platform_key(platform) + try: + return COLD_REFRESH_BUDGETS[key] + except KeyError as error: + raise SnapshotError(f'Missing cold-refresh budget for {key}') from error + + +def cold_refresh_legacy_limit(platform: str) -> float: + key = platform_key(platform) + try: + return COLD_REFRESH_LEGACY_LIMIT_MS[key] + except KeyError as error: + raise SnapshotError(f'Missing legacy cold-refresh ceiling for {key}') from error + + +def cold_refresh_value(snapshot: dict[str, Any], source: str) -> float: + value = performance_value(snapshot, COLD_REFRESH_SPEC, source) + for spec in COLD_DIAGNOSTIC_SPECS: + performance_value(snapshot, spec, source) + return value + + def compare_performance( current: dict[str, Any], baseline: dict[str, Any], platform: str -) -> tuple[list[MetricComparison], list[str]]: +) -> tuple[list[PerformanceComparison], list[str]]: + current_version = performance_schema_version(current, 'current') + baseline_version = performance_schema_version(baseline, 'baseline') + if current_version < baseline_version: + raise SnapshotError( + f'Current performance schema {current_version} is older than baseline schema ' + f'{baseline_version}' + ) + current_envs = require_integer(current.get('environments_count'), 'current.environments_count', minimum=1) baseline_envs = require_integer(baseline.get('environments_count'), 'baseline.environments_count', minimum=1) current_managers = require_integer(current.get('managers_count'), 'current.managers_count') @@ -165,7 +241,7 @@ def compare_performance( if current_managers != baseline_managers: failures.append(f'Manager inventory changed: current={current_managers}, baseline={baseline_managers}') - comparisons = [ + comparisons: list[PerformanceComparison] = [ MetricComparison( spec.label, performance_value(current, spec, 'current'), @@ -174,11 +250,39 @@ def compare_performance( ) for spec, budget in performance_specs(platform) ] - failures.extend( - f'{comparison.label} regressed by {comparison.delta:.0f}ms ({comparison.percent_change:.1f}%)' - for comparison in comparisons - if comparison.regressed - ) + if current_version >= 2: + current_cold = cold_refresh_value(current, 'current') + if baseline_version >= 2: + comparisons.append( + MetricComparison( + COLD_REFRESH_SPEC.label, + current_cold, + cold_refresh_value(baseline, 'baseline'), + cold_refresh_budget(platform), + ) + ) + else: + comparisons.append( + AbsoluteLimitComparison( + COLD_REFRESH_SPEC.label, + current_cold, + cold_refresh_legacy_limit(platform), + ) + ) + + for comparison in comparisons: + if not comparison.regressed: + continue + if isinstance(comparison, MetricComparison): + failures.append( + f'{comparison.label} regressed by {comparison.delta:.0f}ms ' + f'({comparison.percent_change:.1f}%)' + ) + else: + failures.append( + f'{comparison.label} exceeded the legacy-baseline ceiling by ' + f'{comparison.delta:.0f}ms' + ) return comparisons, failures @@ -244,19 +348,30 @@ def status_icon(failed: bool, delta: float) -> str: def performance_report( platform: str, - comparisons: Sequence[MetricComparison], + comparisons: Sequence[PerformanceComparison], failures: Sequence[str], current: dict[str, Any], baseline: dict[str, Any], ) -> str: rows = [] + has_legacy_cold_baseline = False for comparison in comparisons: - rows.append( - f'| {comparison.label} | {comparison.current:.0f}ms | {comparison.baseline:.0f}ms | ' - f'{comparison.delta:+.0f}ms | {comparison.percent_change:+.1f}% | ' - f'>{comparison.budget.absolute_ms:.0f}ms and >{comparison.budget.relative_percent:.0f}% | ' - f"{status_icon(comparison.regressed, comparison.delta)} |" - ) + if isinstance(comparison, MetricComparison): + rows.append( + f'| {comparison.label} | {comparison.current:.0f}ms | ' + f'{comparison.baseline:.0f}ms | {comparison.delta:+.0f}ms | ' + f'{comparison.percent_change:+.1f}% | ' + f'>{comparison.budget.absolute_ms:.0f}ms and ' + f'>{comparison.budget.relative_percent:.0f}% | ' + f"{status_icon(comparison.regressed, comparison.delta)} |" + ) + else: + has_legacy_cold_baseline = True + rows.append( + f'| {comparison.label} | {comparison.current:.0f}ms | legacy schema | n/a | n/a | ' + f'>{comparison.limit:.0f}ms absolute | ' + f"{status_icon(comparison.regressed, comparison.delta)} |" + ) result = ':x: Regression detected' if failures else ':white_check_mark: Within regression budgets' report = [ f'## Performance Report ({platform})', @@ -272,6 +387,11 @@ def performance_report( f"| Environments | {current['environments_count']} | {baseline['environments_count']} |", f"| Managers | {current['managers_count']} | {baseline['managers_count']} |", ] + if has_legacy_cold_baseline: + report.extend([ + '', + '> Cold refresh uses a platform absolute ceiling while the exact base has legacy metrics.', + ]) if failures: report.extend(['', '### Blocking findings', *[f'- {failure}' for failure in failures]]) report.extend([ diff --git a/scripts/tests/test_quality_snapshot.py b/scripts/tests/test_quality_snapshot.py index 97ad8562..fb7fb350 100644 --- a/scripts/tests/test_quality_snapshot.py +++ b/scripts/tests/test_quality_snapshot.py @@ -16,6 +16,7 @@ compare_coverage, compare_performance, load_json, + performance_report, performance_specs, run_coverage, run_performance, @@ -24,9 +25,11 @@ def performance_snapshot( *, refresh_p50=100, refresh_p95=500, startup_p50=10, startup_p95=20, - first_p50=15, first_p95=30, environments=5, managers=1 + first_p50=15, first_p95=30, cold_p50=200, cold_p95=500, + cold_first_p50=25, cold_first_p95=50, environments=5, managers=1, + schema_version=1 ): - return { + snapshot = { 'server_startup_ms': startup_p50, 'full_refresh_ms': refresh_p50, 'time_to_first_env_ms': first_p50, @@ -38,6 +41,21 @@ def performance_snapshot( 'time_to_first_env': {'count': 10, 'p50': first_p50, 'p95': first_p95}, }, } + if schema_version >= 2: + snapshot['metrics_schema_version'] = schema_version + snapshot['cold_refresh_ms'] = cold_p50 + snapshot['cold_time_to_first_env_ms'] = cold_first_p50 + snapshot['stats']['cold_refresh'] = { + 'count': 10, + 'p50': cold_p50, + 'p95': cold_p95, + } + snapshot['stats']['cold_time_to_first_env'] = { + 'count': 10, + 'p50': cold_first_p50, + 'p95': cold_first_p95, + } + return snapshot def write_lcov(path, *, lines_hit, lines_found, functions_hit, functions_found): @@ -64,6 +82,82 @@ def test_p95_regression_fails_even_when_p50_is_unchanged(self): _, failures = compare_performance(current, performance_snapshot(refresh_p95=500), 'Windows') self.assertTrue(any('Full refresh P95' in failure for failure in failures)) + def test_schema_v2_compares_warm_and_cold_metrics(self): + current = performance_snapshot(schema_version=2) + baseline = performance_snapshot(schema_version=2) + + comparisons, failures = compare_performance(current, baseline, 'Windows') + + self.assertEqual(len(comparisons), 7) + self.assertEqual(failures, []) + + def test_schema_v2_requires_cold_samples(self): + current = performance_snapshot(schema_version=2) + del current['stats']['cold_refresh'] + + with self.assertRaisesRegex(SnapshotError, 'current.stats.cold_refresh'): + compare_performance(current, performance_snapshot(), 'Windows') + + def test_schema_v2_requires_cold_diagnostic_percentiles(self): + current = performance_snapshot(schema_version=2) + del current['stats']['cold_time_to_first_env']['p95'] + + with self.assertRaisesRegex(SnapshotError, 'cold_time_to_first_env.p95'): + compare_performance(current, performance_snapshot(), 'Windows') + + def test_legacy_baseline_uses_absolute_cold_ceiling(self): + current = performance_snapshot(schema_version=2, cold_p50=499) + + comparisons, failures = compare_performance(current, performance_snapshot(), 'Linux') + + self.assertEqual(len(comparisons), 7) + self.assertEqual(comparisons[-1].label, 'Cold refresh P50') + self.assertEqual(failures, []) + + def test_legacy_cold_ceiling_is_explicit_in_report(self): + current = performance_snapshot(schema_version=2, cold_p50=499) + baseline = performance_snapshot() + comparisons, failures = compare_performance(current, baseline, 'Linux') + + report = performance_report('Linux', comparisons, failures, current, baseline) + + self.assertIn('| Cold refresh P50 | 499ms | legacy schema |', report) + self.assertIn( + 'Cold refresh uses a platform absolute ceiling while the exact base has legacy metrics.', + report, + ) + + def test_legacy_baseline_rejects_excessive_cold_p50(self): + current = performance_snapshot(schema_version=2, cold_p50=501) + + _, failures = compare_performance(current, performance_snapshot(), 'Linux') + + self.assertTrue(any('Cold refresh P50 exceeded' in failure for failure in failures)) + + def test_cold_p50_regression_fails_when_all_cold_samples_are_slow(self): + current = performance_snapshot(schema_version=2, cold_p50=400) + baseline = performance_snapshot(schema_version=2, cold_p50=150) + + _, failures = compare_performance(current, baseline, 'Windows') + + self.assertTrue(any('Cold refresh P50 regressed' in failure for failure in failures)) + + def test_legacy_current_is_invalid_against_schema_v2_baseline(self): + with self.assertRaisesRegex(SnapshotError, 'older than baseline schema'): + compare_performance( + performance_snapshot(), + performance_snapshot(schema_version=2), + 'Windows', + ) + + def test_newer_performance_schema_is_invalid(self): + with self.assertRaisesRegex(SnapshotError, 'newer than supported version'): + compare_performance( + performance_snapshot(schema_version=3), + performance_snapshot(schema_version=2), + 'Windows', + ) + def test_post_fix_macos_tail_variance_passes(self): baseline = performance_snapshot(startup_p95=621, refresh_p95=1_343, first_p95=649) current = performance_snapshot(startup_p95=691, refresh_p95=1_435, first_p95=745) From 6fd50f120d3325236f8388d29a1d233e961b8f1d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 09:28:38 -0700 Subject: [PATCH 7/8] test: fail fast on missing refresh notifications (PR #510) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/tests/e2e_performance.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/pet/tests/e2e_performance.rs b/crates/pet/tests/e2e_performance.rs index cd286980..8c8df53f 100644 --- a/crates/pet/tests/e2e_performance.rs +++ b/crates/pet/tests/e2e_performance.rs @@ -1445,9 +1445,13 @@ fn test_performance_summary() { "Cold refresh", iteration, ); - if let Some(ttfe) = cold_client.time_to_first_env() { - cold_time_to_first_env_stats.add(ttfe.as_millis()); - } + let cold_ttfe = cold_client.time_to_first_env().unwrap_or_else(|| { + panic!( + "Cold refresh iteration {} produced no environment notification", + iteration + 1 + ) + }); + cold_time_to_first_env_stats.add(cold_ttfe.as_millis()); record_interpreter_probe_timeouts(&cold_client, &mut probe_timeout_counts); println!( @@ -1480,9 +1484,13 @@ fn test_performance_summary() { "Warm refresh", iteration, ); - if let Some(ttfe) = warm_client.time_to_first_env() { - warm_time_to_first_env_stats.add(ttfe.as_millis()); - } + let warm_ttfe = warm_client.time_to_first_env().unwrap_or_else(|| { + panic!( + "Warm refresh iteration {} produced no environment notification", + iteration + 1 + ) + }); + warm_time_to_first_env_stats.add(warm_ttfe.as_millis()); record_interpreter_probe_timeouts(&warm_client, &mut probe_timeout_counts); println!( From 9bc387fe4e823085dddf9f4a3d3e141605818eeb Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 11 Aug 2026 09:37:47 -0700 Subject: [PATCH 8/8] test: isolate performance cache sandboxes (PR #510) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/tests/e2e_performance.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/pet/tests/e2e_performance.rs b/crates/pet/tests/e2e_performance.rs index 8c8df53f..a104a8cb 100644 --- a/crates/pet/tests/e2e_performance.rs +++ b/crates/pet/tests/e2e_performance.rs @@ -547,10 +547,11 @@ fn get_pet_executable() -> PathBuf { } /// Get a temporary cache directory for tests -fn get_test_cache_dir() -> PathBuf { +fn get_test_cache_dir(test_name: &str) -> PathBuf { let tmp = env::temp_dir(); tmp.join("pet-e2e-perf-tests") .join(format!("cache-{}", std::process::id())) + .join(test_name) } fn benchmark_iteration_cache_dir(cache_root: &Path, workload: &str, iteration: usize) -> PathBuf { @@ -907,6 +908,11 @@ fn benchmark_cache_directories_are_isolated_by_workload_and_iteration() { assert_eq!(first_cold, root.join("cold").join("iteration-1")); assert_ne!(first_cold, second_cold); assert_ne!(first_cold, first_warm); + + assert_ne!( + get_test_cache_dir("first-test"), + get_test_cache_dir("second-test") + ); } // ============================================================================ @@ -920,7 +926,7 @@ fn test_server_startup_performance() { let mut configure_stats = StatisticalMetrics::new(); let mut total_stats = StatisticalMetrics::new(); - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("server-startup"); let workspace_dir = get_workspace_dir(); println!( @@ -991,7 +997,7 @@ fn test_full_refresh_performance() { let mut manager_count = 0usize; let mut kind_counts: HashMap = HashMap::new(); - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("full-refresh"); let workspace_dir = get_workspace_dir(); println!( @@ -1079,7 +1085,7 @@ fn test_workspace_scoped_refresh_performance() { let mut client_duration_stats = StatisticalMetrics::new(); let mut env_count = 0usize; - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("workspace-refresh"); let workspace_dir = get_workspace_dir(); println!( @@ -1135,7 +1141,7 @@ fn test_workspace_scoped_refresh_performance() { #[cfg_attr(feature = "ci-perf", test)] #[allow(dead_code)] fn test_kind_specific_refresh_performance() { - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("kind-refresh"); let workspace_dir = get_workspace_dir(); // Test different environment kinds @@ -1203,7 +1209,7 @@ fn test_resolve_performance() { let mut cold_resolve_stats = StatisticalMetrics::new(); let mut warm_resolve_stats = StatisticalMetrics::new(); - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("resolve"); let workspace_dir = get_workspace_dir(); println!( @@ -1306,7 +1312,7 @@ fn test_resolve_performance() { fn test_concurrent_resolve_performance() { let mut client = PetClient::spawn().expect("Failed to spawn server"); - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("concurrent-resolve"); let workspace_dir = get_workspace_dir(); let config = json!({ @@ -1351,7 +1357,7 @@ fn test_concurrent_resolve_performance() { #[allow(dead_code)] fn test_refresh_warm_vs_cold_cache() { // Clean cache directory - let cache_dir = get_test_cache_dir(); + let cache_dir = get_test_cache_dir("warm-vs-cold"); let _ = std::fs::remove_dir_all(&cache_dir); std::fs::create_dir_all(&cache_dir).expect("Failed to create cache dir"); @@ -1405,7 +1411,7 @@ fn test_performance_summary() { let mut probe_timeout_counts: BTreeMap = BTreeMap::new(); let mut expected_inventory = None; - let cache_root = get_test_cache_dir(); + let cache_root = get_test_cache_dir("performance-summary"); reset_cache_dir(&cache_root); let workspace_dir = get_workspace_dir();