From 2cef6b81ed9f28e3e3ca3fe9504338a130659ea5 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:24:21 +0100 Subject: [PATCH 1/2] perf(native): evaluate model contracts without repeated failing type deserialization Scalars skip the Expression deserialization that always failed and built a large error per call, and contract evaluation normalizes each type string once per request. Adds contracts_cpu_ms to compile timings and a same-runner python vs native-preview gate on it. --- .github/workflows/verify.yml | 28 ++++++- .../src/contracts/_helpers/columns.rs | 12 +-- .../src/contracts/_helpers/dynamic.rs | 6 +- .../src/contracts/_helpers/type_comparison.rs | 58 ++++++++++----- .../main/evaluate_model_contracts.rs | 4 +- .../tests/test_evaluate_model_contracts.rs | 61 ++++++++++++++- .../src/contracts/tests/test_types.rs | 7 ++ .../src/type_system/_helpers/parsed.rs | 5 +- .../_helpers/measure.py | 33 ++++++--- .../_helpers/report.py | 12 +++ .../_helpers/verdict.py | 33 +++++++++ .../compile_performance_ratio/constants.py | 4 + .../compile_performance_ratio/main/compare.py | 54 ++++++++++++-- .../cli/commands/_helpers/compile/pipeline.py | 3 + .../cli/commands/main/project/_compile.py | 1 + src/sqlbuild/cli/compile/models.py | 1 + .../compile_performance_ratio/_test_types.py | 9 +++ .../test_compare_compile_performance.py | 74 +++++++++++++++++++ .../main/build/test_compile_json_behavior.py | 1 + 19 files changed, 358 insertions(+), 48 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index ccc2a8763d..28478a2601 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -585,7 +585,7 @@ jobs: run: make test-e2e-diagnostic-compile-performance compile_performance_ratio: - name: Same-runner compile ratio (${{ matrix.kind }} 3000 models) + name: Same-runner compile ratio (${{ matrix.label || matrix.kind }} 3000 models) needs: [changes, lockfile] if: needs.changes.outputs.performance == 'true' runs-on: ubuntu-24.04 @@ -598,6 +598,12 @@ jobs: max_ratio: "1.10" - kind: fresh max_ratio: "1.10" + - kind: dense + label: native-preview contracts vs python + base_engine: python + head_engine: native-preview + gate_phase: contracts_cpu_ms + max_ratio: "1.00" steps: - uses: actions/checkout@v4 - name: Check out the comparison base @@ -624,7 +630,27 @@ jobs: - name: Install base dependencies working-directory: base run: uv sync --locked --all-groups --all-extras + - name: Compare python and native-preview contract evaluation CPU on head + if: matrix.gate_phase + env: + KIND: ${{ matrix.kind }} + MAX_RATIO: ${{ matrix.max_ratio }} + BASE_ENGINE: ${{ matrix.base_engine }} + HEAD_ENGINE: ${{ matrix.head_engine }} + GATE_PHASE: ${{ matrix.gate_phase }} + run: | + uv run python -m scripts.compare_compile_performance \ + --kind "${KIND}" \ + --modes cold \ + --max-ratio "${MAX_RATIO}" \ + --models 3000 \ + --base-python .venv/bin/python \ + --head-python .venv/bin/python \ + --base-engine "${BASE_ENGINE}" \ + --head-engine "${HEAD_ENGINE}" \ + --gate-phase "${GATE_PHASE}" - name: Compare base and head cold, warm and one-model edit compiles + if: ${{ !matrix.gate_phase }} env: KIND: ${{ matrix.kind }} MAX_RATIO: ${{ matrix.max_ratio }} diff --git a/crates/sqlbuild-analysis/src/contracts/_helpers/columns.rs b/crates/sqlbuild-analysis/src/contracts/_helpers/columns.rs index 7e7fda68b8..20922fe52b 100644 --- a/crates/sqlbuild-analysis/src/contracts/_helpers/columns.rs +++ b/crates/sqlbuild-analysis/src/contracts/_helpers/columns.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use crate::contracts::_helpers::dynamic::dynamic_column_diagnostics; -use crate::contracts::_helpers::type_comparison::{TypeComparison, types_equal}; +use crate::contracts::_helpers::type_comparison::{TypeComparer, TypeComparison}; use crate::contracts::constants::{ CONTRACT_ENFORCED, CONTRACT_NONE, DYNAMIC_OUTPUT_NOT_PROVEN_CODE, EXTRA_COLUMN_CODE, EXTRA_COLUMN_HELP, EXTRA_COLUMN_NAMED_SCHEMA_HELP, MISSING_COLUMN_CODE, @@ -45,7 +45,7 @@ pub(crate) fn declared_shape_validation_is_active(model: &ContractModel, implici pub(crate) fn model_contract_diagnostics( model: &ContractModel, validate_declared_shape: bool, - dialect: &str, + types: &TypeComparer<'_>, ) -> Result, ContractDeferral> { let contract_enforced: bool = model.contract.as_deref() == Some(CONTRACT_ENFORCED); let Some(schema) = model @@ -59,7 +59,7 @@ pub(crate) fn model_contract_diagnostics( Vec::new() }); }; - let mut diagnostics: Vec = dynamic_column_diagnostics(model, dialect)?; + let mut diagnostics: Vec = dynamic_column_diagnostics(model, types)?; let dynamic_output_unproven: bool = diagnostics .iter() .any(|diagnostic| diagnostic.code == DYNAMIC_OUTPUT_NOT_PROVEN_CODE); @@ -107,7 +107,7 @@ pub(crate) fn model_contract_diagnostics( type_enforcement: schema.type_enforcement, contract_enforced, unchecked: &unchecked, - dialect, + types, }; diagnostics.extend(column_type(&checked, &check)?); } @@ -118,7 +118,7 @@ struct TypeCheck<'a> { type_enforcement: bool, contract_enforced: bool, unchecked: &'a HashSet<&'a str>, - dialect: &'a str, + types: &'a TypeComparer<'a>, } struct TypedColumn<'a> { @@ -271,7 +271,7 @@ fn column_type( help: UNKNOWN_TYPE_HELP.to_owned(), })); }; - if types_equal(declared_type, inferred_type, check.dialect)? == TypeComparison::Equal { + if check.types.types_equal(declared_type, inferred_type)? == TypeComparison::Equal { return Ok(None); } Ok(Some(ContractDiagnostic { diff --git a/crates/sqlbuild-analysis/src/contracts/_helpers/dynamic.rs b/crates/sqlbuild-analysis/src/contracts/_helpers/dynamic.rs index b42f5df666..058b485381 100644 --- a/crates/sqlbuild-analysis/src/contracts/_helpers/dynamic.rs +++ b/crates/sqlbuild-analysis/src/contracts/_helpers/dynamic.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; -use crate::contracts::_helpers::type_comparison::{TypeComparison, types_equal}; +use crate::contracts::_helpers::type_comparison::{TypeComparer, TypeComparison}; use crate::contracts::constants::{ DYNAMIC_FAMILY_TYPE_MISMATCH_HELP, DYNAMIC_FAMILY_UNKNOWN_TYPE_HELP, DYNAMIC_OUTPUT_NOT_PROVEN_CODE, DYNAMIC_OUTPUT_NOT_PROVEN_HELP, NO_COMPILER_EVIDENCE, @@ -16,7 +16,7 @@ use crate::contracts::models::{ /// The dynamic column contract diagnostics of one model, before its fixed columns. pub(crate) fn dynamic_column_diagnostics( model: &ContractModel, - dialect: &str, + types: &TypeComparer<'_>, ) -> Result, ContractDeferral> { let Some(schema) = model .schema @@ -49,7 +49,7 @@ pub(crate) fn dynamic_column_diagnostics( )); continue; }; - if types_equal(declared, inferred, dialect)? == TypeComparison::Different { + if types.types_equal(declared, inferred)? == TypeComparison::Different { diagnostics.push(family_diagnostic( TYPE_MISMATCH_CODE, ContractSeverity::Error, diff --git a/crates/sqlbuild-analysis/src/contracts/_helpers/type_comparison.rs b/crates/sqlbuild-analysis/src/contracts/_helpers/type_comparison.rs index 9089685e7e..13bb94ae69 100644 --- a/crates/sqlbuild-analysis/src/contracts/_helpers/type_comparison.rs +++ b/crates/sqlbuild-analysis/src/contracts/_helpers/type_comparison.rs @@ -1,5 +1,8 @@ //! Python's `types_equal` over the native type system, deferring where it cannot answer. +use std::cell::RefCell; +use std::collections::HashMap; + use crate::contracts::models::ContractDeferral; use crate::type_system::main::normalize_type::normalize_type; use crate::type_system::models::{NormalizedType, TypeNormalization}; @@ -11,23 +14,44 @@ pub(crate) enum TypeComparison { Different, } -/// Compare two types as Python's `types_equal`, or defer when either type has no native answer. -pub(crate) fn types_equal( - left: &str, - right: &str, - dialect: &str, -) -> Result { - let left: NormalizedType = normalized(left, dialect)?; - let right: NormalizedType = normalized(right, dialect)?; - Ok(if left == right { - TypeComparison::Equal - } else { - TypeComparison::Different - }) +/// One request's type comparisons, normalizing each type string once as Python's cache does. +pub(crate) struct TypeComparer<'a> { + dialect: &'a str, + normalized: RefCell>>, } -fn normalized(type_sql: &str, dialect: &str) -> Result { - normalize_type(type_sql, dialect) - .map(|TypeNormalization { normalized, .. }| normalized) - .ok_or(ContractDeferral::TypeNormalization) +impl<'a> TypeComparer<'a> { + pub(crate) fn new(dialect: &'a str) -> Self { + Self { + dialect, + normalized: RefCell::new(HashMap::new()), + } + } + + /// Compare two types as Python's `types_equal`, or defer when either has no native answer. + pub(crate) fn types_equal( + &self, + left: &str, + right: &str, + ) -> Result { + let left: NormalizedType = self.normalized(left)?; + let right: NormalizedType = self.normalized(right)?; + Ok(if left == right { + TypeComparison::Equal + } else { + TypeComparison::Different + }) + } + + fn normalized(&self, type_sql: &str) -> Result { + if let Some(known) = self.normalized.borrow().get(type_sql) { + return known.clone().ok_or(ContractDeferral::TypeNormalization); + } + let normalized: Option = normalize_type(type_sql, self.dialect) + .map(|TypeNormalization { normalized, .. }| normalized); + self.normalized + .borrow_mut() + .insert(type_sql.to_owned(), normalized.clone()); + normalized.ok_or(ContractDeferral::TypeNormalization) + } } diff --git a/crates/sqlbuild-analysis/src/contracts/main/evaluate_model_contracts.rs b/crates/sqlbuild-analysis/src/contracts/main/evaluate_model_contracts.rs index b70090de85..a1fa939fbf 100644 --- a/crates/sqlbuild-analysis/src/contracts/main/evaluate_model_contracts.rs +++ b/crates/sqlbuild-analysis/src/contracts/main/evaluate_model_contracts.rs @@ -3,12 +3,14 @@ use crate::contracts::_helpers::columns::{ declared_shape_validation_is_active, model_contract_diagnostics, requires_contract_evaluation, }; +use crate::contracts::_helpers::type_comparison::TypeComparer; use crate::contracts::models::{ContractOutcome, ContractRequest}; /// One outcome per requested model; models that need no evaluation have no diagnostics. #[must_use] pub fn evaluate_model_contracts(request: &ContractRequest) -> Vec { let implicit: bool = request.implicit_column_contracts; + let types = TypeComparer::new(&request.dialect); request .models .iter() @@ -19,7 +21,7 @@ pub fn evaluate_model_contracts(request: &ContractRequest) -> Vec ContractOutcome::Diagnostics(diagnostics), Err(deferral) => ContractOutcome::Deferred(deferral), diff --git a/crates/sqlbuild-analysis/src/contracts/tests/test_evaluate_model_contracts.rs b/crates/sqlbuild-analysis/src/contracts/tests/test_evaluate_model_contracts.rs index fa83f9ded2..2b3883d3e1 100644 --- a/crates/sqlbuild-analysis/src/contracts/tests/test_evaluate_model_contracts.rs +++ b/crates/sqlbuild-analysis/src/contracts/tests/test_evaluate_model_contracts.rs @@ -1,9 +1,9 @@ use crate::contracts::main::evaluate_model_contracts::evaluate_model_contracts; -use crate::contracts::models::{ContractOutcome, ContractRequest}; +use crate::contracts::models::{ContractModel, ContractOutcome, ContractRequest}; use crate::contracts::tests::helpers::{ declared, dynamic_model, inferred, model, outcome_lines, proof, schema, with_inferred, }; -use crate::contracts::tests::test_types::ContractTestCase; +use crate::contracts::tests::test_types::{ContractTestCase, SharedTypesTestCase}; #[test] fn given_model_contracts_when_evaluating_then_diagnostics_follow_python_order() { @@ -210,3 +210,60 @@ fn given_model_contracts_when_evaluating_then_diagnostics_follow_python_order() ); } } + +#[test] +fn given_models_sharing_types_when_evaluating_together_then_each_matches_its_own_request() { + let enforced = |declared_type: &str, inferred_type: &str| { + with_inferred( + model( + Some("enforced"), + Some(schema( + vec![declared("amount", Some(declared_type), false)], + false, + )), + ), + vec![inferred("amount", Some(inferred_type), false)], + ) + }; + let test_cases = [SharedTypesTestCase { + description: "repeated equal, mismatched and unanswerable types", + models: vec![ + enforced("INT", "INTEGER"), + enforced("INT", "VARCHAR"), + enforced("TÉXT", "VARCHAR"), + enforced("INT", "INTEGER"), + enforced("INT", "VARCHAR"), + enforced("TÉXT", "VARCHAR"), + ], + expected_lines: &[ + "K002 error declared:0 column 'amount' inferred as VARCHAR but declared type is INT [amount: inferred VARCHAR]", + "deferred:type_normalization", + "K002 error declared:0 column 'amount' inferred as VARCHAR but declared type is INT [amount: inferred VARCHAR]", + "deferred:type_normalization", + ], + }]; + for test_case in test_cases { + let request = |models: Vec| ContractRequest { + dialect: "duckdb".to_owned(), + implicit_column_contracts: true, + models, + }; + let together: Vec = evaluate_model_contracts(&request(test_case.models.clone())) + .iter() + .flat_map(outcome_lines) + .collect(); + let alone: Vec = test_case + .models + .iter() + .flat_map(|model| evaluate_model_contracts(&request(vec![model.clone()]))) + .flat_map(|outcome| outcome_lines(&outcome)) + .collect(); + + assert_eq!(together, alone, "{}", test_case.description); + assert_eq!( + together, test_case.expected_lines, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/contracts/tests/test_types.rs b/crates/sqlbuild-analysis/src/contracts/tests/test_types.rs index bfbdb73893..43a0260eb6 100644 --- a/crates/sqlbuild-analysis/src/contracts/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/contracts/tests/test_types.rs @@ -8,6 +8,13 @@ pub(crate) struct ContractTestCase { pub(crate) expected_lines: &'static [&'static str], } +pub(crate) struct SharedTypesTestCase { + pub(crate) description: &'static str, + pub(crate) models: Vec, + /// Every model's lines in order, as each model's own single-model request yields them. + pub(crate) expected_lines: &'static [&'static str], +} + pub(crate) struct PromotionTestCase { pub(crate) description: &'static str, pub(crate) request: PromotionRequest, diff --git a/crates/sqlbuild-analysis/src/type_system/_helpers/parsed.rs b/crates/sqlbuild-analysis/src/type_system/_helpers/parsed.rs index dfc944b99f..dc08a3b5d3 100644 --- a/crates/sqlbuild-analysis/src/type_system/_helpers/parsed.rs +++ b/crates/sqlbuild-analysis/src/type_system/_helpers/parsed.rs @@ -227,8 +227,11 @@ fn python_str(value: &Value) -> Option { } } -/// Whether the wheel hands Python this value as an expression object rather than data. +/// Whether the wheel hands Python an expression object; every variant holds data, so not a scalar. fn is_python_expression(value: &Value) -> bool { + if !matches!(value, Value::Object(_) | Value::Array(_)) { + return false; + } if let Ok(_expression) = expression_from_value(value.clone()) { return true; } diff --git a/scripts/compile_performance_ratio/_helpers/measure.py b/scripts/compile_performance_ratio/_helpers/measure.py index 9931aefcc4..d17e8204a8 100644 --- a/scripts/compile_performance_ratio/_helpers/measure.py +++ b/scripts/compile_performance_ratio/_helpers/measure.py @@ -20,6 +20,7 @@ BASE_LABEL, COLD_MODE, COMPILE_ENTRY, + COMPILER_ENGINE_KEY, DENSE_KIND, EDIT_MODE, ERROR_TAIL_CHARACTERS, @@ -116,34 +117,42 @@ def compare_builds( head_python: Path, runs: int, modes: tuple[str, ...], + engines: tuple[str | None, str | None] = (None, None), ) -> tuple[CompileComparison, ...]: """Alternate base and head compiles per mode; warm and edit reuse the cold project.""" - builds: tuple[tuple[str, Path, Path], ...] = ( - (BASE_LABEL, base_python, base_project_dir), - (HEAD_LABEL, head_python, head_project_dir), + base_engine, head_engine = engines + builds: tuple[tuple[str, Path, Path, str | None], ...] = ( + (BASE_LABEL, base_python, base_project_dir, base_engine), + (HEAD_LABEL, head_python, head_project_dir, head_engine), ) comparisons: list[CompileComparison] = [] cache_primed: bool = False for mode in modes: print(f"Comparing {MODE_TITLES[mode]} over {runs} alternating runs", file=sys.stderr) if mode == COLD_MODE: - for label, python, project_dir in builds: - _ = _compile_once(label=label, python=python, project_dir=project_dir, mode=mode) + for label, python, project_dir, engine in builds: + _ = _compile_once( + label=label, python=python, project_dir=project_dir, mode=mode, engine=engine + ) cache_primed = False elif not cache_primed: - for label, python, project_dir in builds: + for label, python, project_dir, engine in builds: _ = _compile_once( - label=label, python=python, project_dir=project_dir, mode=WARM_MODE + label=label, + python=python, + project_dir=project_dir, + mode=WARM_MODE, + engine=engine, ) cache_primed = True results: dict[str, list[CompileRun]] = {BASE_LABEL: [], HEAD_LABEL: []} for revision in range(runs): - for label, python, project_dir in builds: + for label, python, project_dir, engine in builds: if mode == EDIT_MODE: _ = apply_one_model_edit(project_dir=project_dir, revision=revision) run: CompileRun = _compile_once( - label=label, python=python, project_dir=project_dir, mode=mode + label=label, python=python, project_dir=project_dir, mode=mode, engine=engine ) _check_cache_use(run=run, mode=mode) results[label].append(run) @@ -191,12 +200,16 @@ def _check_cache_use(*, run: CompileRun, mode: str) -> None: ) -def _compile_once(*, label: str, python: Path, project_dir: Path, mode: str) -> CompileRun: +def _compile_once( + *, label: str, python: Path, project_dir: Path, mode: str, engine: str | None +) -> CompileRun: cache_args: tuple[str, ...] = () if mode == COLD_MODE: shutil.rmtree(project_dir / "target", ignore_errors=True) cache_args = ("--no-cache",) environment: dict[str, str] = _compile_environment() + if engine is not None: + environment[COMPILER_ENGINE_KEY] = engine before: resource.struct_rusage = resource.getrusage(resource.RUSAGE_CHILDREN) started: float = time.perf_counter() completed: subprocess.CompletedProcess[str] = subprocess.run( diff --git a/scripts/compile_performance_ratio/_helpers/report.py b/scripts/compile_performance_ratio/_helpers/report.py index 84cdd7589e..2cc764acf3 100644 --- a/scripts/compile_performance_ratio/_helpers/report.py +++ b/scripts/compile_performance_ratio/_helpers/report.py @@ -18,6 +18,8 @@ def comparison_markdown( per_side_projects: bool, failures: tuple[str, ...], mode_max_ratios: Mapping[str, float] | None = None, + engines: tuple[str | None, str | None] = (None, None), + gate_phases: tuple[str, ...] = (), ) -> str: """Summarize medians, ratios and per-phase timings of every mode as Markdown tables.""" @@ -27,6 +29,16 @@ def comparison_markdown( else "One project generated with head's generator and compiled by both builds." ) lines: list[str] = [] + base_engine, head_engine = engines + if base_engine is not None or head_engine is not None: + lines.extend( + ( + f"Engines: base `{base_engine or 'default'}`, head `{head_engine or 'default'}`.", + "", + ) + ) + if gate_phases: + lines.extend((f"Gated phases: {', '.join(gate_phases)}.", "")) for comparison in comparisons: lines.extend( ( diff --git a/scripts/compile_performance_ratio/_helpers/verdict.py b/scripts/compile_performance_ratio/_helpers/verdict.py index cca469c30b..4267778723 100644 --- a/scripts/compile_performance_ratio/_helpers/verdict.py +++ b/scripts/compile_performance_ratio/_helpers/verdict.py @@ -47,5 +47,38 @@ def ratio_failures( return tuple(failures) +def phase_failures( + *, + comparisons: tuple[CompileComparison, ...], + modes: tuple[str, ...], + phases: tuple[str, ...], + max_ratio: float, + noise_floor_ms: float, +) -> tuple[str, ...]: + """Describe every gated phase that is unmeasured or slower than ratio plus floor allow.""" + + measured: dict[str, CompileComparison] = { + comparison.mode: comparison for comparison in comparisons + } + failures: list[str] = [] + for mode in modes: + comparison: CompileComparison | None = measured.get(mode) + if comparison is None: + failures.append(f"{mode}: no measurement") + continue + for phase in phases: + prefix: str = f"{comparison.kind} {comparison.models} {mode} {phase}" + base: float | None = comparison.base_timings_ms.get(phase) + head: float | None = comparison.head_timings_ms.get(phase) + if base is None or head is None: + failures.append(f"{prefix}: not reported") + elif head > base * max_ratio + noise_floor_ms: + failures.append( + f"{prefix}: {head:.0f} ms exceeds {max_ratio:.2f}x base {base:.0f} ms " + f"plus {noise_floor_ms:.0f} ms" + ) + return tuple(failures) + + def _measured(seconds: float) -> bool: return math.isfinite(seconds) and seconds > 0 diff --git a/scripts/compile_performance_ratio/constants.py b/scripts/compile_performance_ratio/constants.py index ca1301ec6a..d51fa2cd83 100644 --- a/scripts/compile_performance_ratio/constants.py +++ b/scripts/compile_performance_ratio/constants.py @@ -34,6 +34,8 @@ "kind=sys.argv[1], project_dir=Path(sys.argv[2]), models=int(sys.argv[3]))" ) PYTHONPATH_KEY: str = "PYTHONPATH" +COMPILER_ENGINE_KEY: str = "SQLBUILD_COMPILER_ENGINE" +PHASE_NOISE_FLOOR_MS: float = 50.0 ERROR_TAIL_CHARACTERS: int = 2000 FRESH_SOURCE_SHARE: float = 713 / 3000 FRESH_SEED_SHARE: float = 141 / 3000 @@ -46,6 +48,8 @@ "graph_ms", "model_analysis_ms", "analysis_native_ms", + "contracts_ms", + "contracts_cpu_ms", "built_in_rules_ms", "custom_rules_ms", "test_planning_ms", diff --git a/scripts/compile_performance_ratio/main/compare.py b/scripts/compile_performance_ratio/main/compare.py index 330bc2704d..322780b793 100644 --- a/scripts/compile_performance_ratio/main/compare.py +++ b/scripts/compile_performance_ratio/main/compare.py @@ -14,14 +14,16 @@ write_benchmark_project, ) from scripts.compile_performance_ratio._helpers.report import append_summary, comparison_markdown -from scripts.compile_performance_ratio._helpers.verdict import ratio_failures +from scripts.compile_performance_ratio._helpers.verdict import phase_failures, ratio_failures from scripts.compile_performance_ratio.constants import ( COMPILE_MODES, DEFAULT_MAX_RATIO, DEFAULT_RUNS, EDIT_MODE, NOISE_FLOOR_SECONDS, + PHASE_NOISE_FLOOR_MS, PROJECT_KINDS, + REPORTED_PHASES, ) from scripts.compile_performance_ratio.models import CompileComparison @@ -59,16 +61,28 @@ def compare_compile_performance(argv: list[str] | None = None) -> int: head_python=args.head_python, runs=args.runs, modes=modes, + engines=(args.base_engine, args.head_engine), ) mode_max_ratios: dict[str, float] = ( {} if args.edit_max_ratio is None else {EDIT_MODE: args.edit_max_ratio} ) - failures: tuple[str, ...] = ratio_failures( - comparisons=comparisons, - modes=modes, - max_ratio=args.max_ratio, - noise_floor_seconds=args.noise_floor_seconds, - mode_max_ratios=mode_max_ratios, + gate_phases: tuple[str, ...] = tuple(dict.fromkeys(args.gate_phases)) + failures: tuple[str, ...] = ( + phase_failures( + comparisons=comparisons, + modes=modes, + phases=gate_phases, + max_ratio=args.max_ratio, + noise_floor_ms=args.phase_noise_floor_ms, + ) + if gate_phases + else ratio_failures( + comparisons=comparisons, + modes=modes, + max_ratio=args.max_ratio, + noise_floor_seconds=args.noise_floor_seconds, + mode_max_ratios=mode_max_ratios, + ) ) markdown: str = comparison_markdown( comparisons=comparisons, @@ -77,6 +91,8 @@ def compare_compile_performance(argv: list[str] | None = None) -> int: per_side_projects=per_side_projects, failures=failures, mode_max_ratios=mode_max_ratios, + engines=(args.base_engine, args.head_engine), + gate_phases=gate_phases, ) print(markdown) _ = append_summary(path=Path(summary_value) if summary_value else None, markdown=markdown) @@ -122,4 +138,28 @@ def _parse_args(argv: list[str] | None) -> argparse.Namespace: default=NOISE_FLOOR_SECONDS, help="Seconds of slack added to the ratio allowance before a slowdown fails.", ) + parser.add_argument( + "--base-engine", + default=None, + help="Compiler engine the base build runs; defaults to the build's own default.", + ) + parser.add_argument( + "--head-engine", + default=None, + help="Compiler engine the head build runs; defaults to the build's own default.", + ) + parser.add_argument( + "--gate-phase", + dest="gate_phases", + action="append", + choices=REPORTED_PHASES, + default=[], + help="Gate only this reported phase's median instead of whole-compile wall and CPU.", + ) + parser.add_argument( + "--phase-noise-floor-ms", + type=float, + default=PHASE_NOISE_FLOOR_MS, + help="Milliseconds of slack added to a gated phase's ratio allowance.", + ) return parser.parse_args(argv) diff --git a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py index 1f21ac3d72..5ecad25968 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py @@ -170,6 +170,7 @@ def _analyze_compile_project( status=status, message=f"Analyzed column lineage. ({lineage_ms / 1000:.2f}s)" ) contracts_start: float = time.monotonic() + contracts_cpu_start: float = time.thread_time() contract_result: ContractValidationResult if profile_flags.skip_contracts: contract_result = ContractValidationResult(diagnostics=()) @@ -180,6 +181,7 @@ def _analyze_compile_project( dialect=adapter.sql_analysis_dialect(), ) contract_ms: int = elapsed_ms(contracts_start) + contract_cpu_ms: int = int((time.thread_time() - contracts_cpu_start) * 1000) if not profile_flags.skip_contracts: _ = complete_compile_phase( status=status, message=f"Validated model contracts. ({contract_ms / 1000:.2f}s)" @@ -255,6 +257,7 @@ def _analyze_compile_project( graph_ms=graph_ms, lineage_ms=lineage_ms, contract_ms=contract_ms, + contract_cpu_ms=contract_cpu_ms, built_in_rules_ms=rules_result.built_in_ms, custom_rules_ms=rules_result.custom_ms, early_lint_wait_ms=early_lint_wait_ms, diff --git a/src/sqlbuild/cli/commands/main/project/_compile.py b/src/sqlbuild/cli/commands/main/project/_compile.py index 7edbb6d3e1..6ea2d58d06 100644 --- a/src/sqlbuild/cli/commands/main/project/_compile.py +++ b/src/sqlbuild/cli/commands/main/project/_compile.py @@ -166,6 +166,7 @@ def _run_compile_with_status( "graph_ms": analysis.graph_ms, "lineage_ms": analysis.lineage_ms, "contracts_ms": analysis.contract_ms, + "contracts_cpu_ms": analysis.contract_cpu_ms, "built_in_rules_ms": analysis.built_in_rules_ms, "custom_rules_ms": analysis.custom_rules_ms, "early_lint_wait_ms": analysis.early_lint_wait_ms, diff --git a/src/sqlbuild/cli/compile/models.py b/src/sqlbuild/cli/compile/models.py index a590ab705e..1a166ca13a 100644 --- a/src/sqlbuild/cli/compile/models.py +++ b/src/sqlbuild/cli/compile/models.py @@ -42,6 +42,7 @@ class CompileAnalysis: graph_ms: int lineage_ms: int contract_ms: int + contract_cpu_ms: int built_in_rules_ms: int = 0 custom_rules_ms: int = 0 early_lint_wait_ms: int = 0 diff --git a/tests/e2e/scripts/compile_performance_ratio/_test_types.py b/tests/e2e/scripts/compile_performance_ratio/_test_types.py index 8f517caae3..4df5ed04c2 100644 --- a/tests/e2e/scripts/compile_performance_ratio/_test_types.py +++ b/tests/e2e/scripts/compile_performance_ratio/_test_types.py @@ -23,3 +23,12 @@ class PerSideCompilePerformanceRatioTestCase: expected_fragments: tuple[str, ...] expected_base_projects: tuple[str, ...] expected_head_projects: tuple[str, ...] + + +@dataclass(frozen=True) +class EnginePhaseRatioTestCase: + description: str + max_ratio: str + phase_noise_floor_ms: str + expected_return_code: int + expected_fragments: tuple[str, ...] diff --git a/tests/e2e/scripts/compile_performance_ratio/test_compare_compile_performance.py b/tests/e2e/scripts/compile_performance_ratio/test_compare_compile_performance.py index fda715ddc8..cfda3aaa37 100644 --- a/tests/e2e/scripts/compile_performance_ratio/test_compare_compile_performance.py +++ b/tests/e2e/scripts/compile_performance_ratio/test_compare_compile_performance.py @@ -11,6 +11,7 @@ from tests.e2e.scripts.compile_performance_ratio._test_types import ( CompilePerformanceRatioTestCase, + EnginePhaseRatioTestCase, PerSideCompilePerformanceRatioTestCase, ) from tests.e2e.scripts.compile_performance_ratio.helpers import ( @@ -167,5 +168,78 @@ def test_given_base_root_when_comparing_compile_performance_then_each_side_uses_ assert logged_projects(head_log) == test_case.expected_head_projects +@pytest.mark.parametrize( + "test_case", + ( + EnginePhaseRatioTestCase( + description="native-preview contracts within a generous limit of python's", + max_ratio="1000", + phase_noise_floor_ms="1000", + expected_return_code=0, + expected_fragments=( + "Engines: base `python`, head `native-preview`.", + "Gated phases: contracts_cpu_ms.", + "| contracts_cpu_ms |", + "**Result: passed.**", + ), + ), + EnginePhaseRatioTestCase( + description="a gated phase over its allowance fails on that phase alone", + max_ratio="0", + phase_noise_floor_ms="-1", + expected_return_code=1, + expected_fragments=( + "Compile performance regression: dense 20 cold contracts_cpu_ms:", + "**Result: failed.**", + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_two_engines_when_gating_a_phase_then_enforces_only_that_phase( + test_case: EnginePhaseRatioTestCase, +) -> None: + environment: dict[str, str] = dict(os.environ) + _ = environment.pop("GITHUB_STEP_SUMMARY", None) + result: subprocess.CompletedProcess[str] = subprocess.run( + [ + sys.executable, + "-m", + "scripts.compare_compile_performance", + "--kind", + "dense", + "--models", + "20", + "--modes", + "cold", + "--runs", + "1", + "--base-python", + sys.executable, + "--head-python", + sys.executable, + "--base-engine", + "python", + "--head-engine", + "native-preview", + "--gate-phase", + "contracts_cpu_ms", + "--max-ratio", + test_case.max_ratio, + "--phase-noise-floor-ms", + test_case.phase_noise_floor_ms, + ], + cwd=_REPO_ROOT, + capture_output=True, + text=True, + env=environment, + check=False, + ) + + output: str = result.stdout + result.stderr + assert result.returncode == test_case.expected_return_code, output + assert all(fragment in output for fragment in test_case.expected_fragments), output + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py index 1bf00f83ce..dff9027895 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py @@ -66,6 +66,7 @@ def test_given_waffle_shop_when_running_compile_json_then_it_reports_offline_que "graph_ms", "lineage_ms", "contracts_ms", + "contracts_cpu_ms", "write_ms", "attachment_ms", "built_in_rules_ms", From 0c455a1b607229f30167676e0c42585a7b587920 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:42:21 +0100 Subject: [PATCH 2/2] test(perf): report a phase missing from any run instead of counting it as zero --- .../_helpers/measure.py | 13 +++-- .../_helpers/_test_types.py | 9 ++- .../_helpers/test_measure.py | 58 +++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 tests/unit/scripts/compile_performance_ratio/_helpers/test_measure.py diff --git a/scripts/compile_performance_ratio/_helpers/measure.py b/scripts/compile_performance_ratio/_helpers/measure.py index d17e8204a8..0f3882c47a 100644 --- a/scripts/compile_performance_ratio/_helpers/measure.py +++ b/scripts/compile_performance_ratio/_helpers/measure.py @@ -179,8 +179,8 @@ def _comparison( head_wall_seconds=statistics.median(run.wall_seconds for run in head), base_cpu_seconds=statistics.median(run.cpu_seconds for run in base), head_cpu_seconds=statistics.median(run.cpu_seconds for run in head), - base_timings_ms=_median_phases(runs=base), - head_timings_ms=_median_phases(runs=head), + base_timings_ms=median_phases(runs=base), + head_timings_ms=median_phases(runs=head), ) @@ -260,9 +260,12 @@ def _compile_environment() -> dict[str, str]: } -def _median_phases(*, runs: list[CompileRun]) -> dict[str, float]: +def median_phases(*, runs: list[CompileRun]) -> dict[str, float]: + """Return each phase's median over runs, omitting phases any run did not report.""" + medians: dict[str, float] = {} for phase in REPORTED_PHASES: - values: list[int] = [run.timings_ms.get(phase, 0) for run in runs] - medians[phase] = statistics.median(values) + values: list[int] = [run.timings_ms[phase] for run in runs if phase in run.timings_ms] + if values and len(values) == len(runs): + medians[phase] = statistics.median(values) return medians diff --git a/tests/unit/scripts/compile_performance_ratio/_helpers/_test_types.py b/tests/unit/scripts/compile_performance_ratio/_helpers/_test_types.py index 459d0a388b..5f2afdd6ce 100644 --- a/tests/unit/scripts/compile_performance_ratio/_helpers/_test_types.py +++ b/tests/unit/scripts/compile_performance_ratio/_helpers/_test_types.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field -from scripts.compile_performance_ratio.models import CompileComparison +from scripts.compile_performance_ratio.models import CompileComparison, CompileRun @dataclass(frozen=True) @@ -37,3 +37,10 @@ class OneModelEditErrorTestCase: description: str model_files: dict[str, str] expected_message: str + + +@dataclass(frozen=True) +class MedianPhasesTestCase: + description: str + runs: tuple[CompileRun, ...] + expected_phases: dict[str, float] diff --git a/tests/unit/scripts/compile_performance_ratio/_helpers/test_measure.py b/tests/unit/scripts/compile_performance_ratio/_helpers/test_measure.py new file mode 100644 index 0000000000..cc7cff4c35 --- /dev/null +++ b/tests/unit/scripts/compile_performance_ratio/_helpers/test_measure.py @@ -0,0 +1,58 @@ +"""Tests for per-phase medians in the same-runner compile performance ratio guard.""" + +import pytest + +from scripts.compile_performance_ratio._helpers.measure import median_phases +from scripts.compile_performance_ratio.models import CompileRun +from tests.unit.scripts.compile_performance_ratio._helpers._test_types import ( + MedianPhasesTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + ( + MedianPhasesTestCase( + description="a phase every run reports gets its median", + runs=( + CompileRun( + label="head", + wall_seconds=1.0, + cpu_seconds=1.0, + timings_ms={"contracts_cpu_ms": 90}, + ), + CompileRun( + label="head", + wall_seconds=1.0, + cpu_seconds=1.0, + timings_ms={"contracts_cpu_ms": 120}, + ), + CompileRun( + label="head", + wall_seconds=1.0, + cpu_seconds=1.0, + timings_ms={"contracts_cpu_ms": 100}, + ), + ), + expected_phases={"contracts_cpu_ms": 100}, + ), + MedianPhasesTestCase( + description="a phase one run omits is left unreported instead of counted as zero", + runs=( + CompileRun( + label="head", + wall_seconds=1.0, + cpu_seconds=1.0, + timings_ms={"contracts_cpu_ms": 90}, + ), + CompileRun(label="head", wall_seconds=1.0, cpu_seconds=1.0, timings_ms={}), + ), + expected_phases={}, + ), + ), + ids=lambda case: case.description, +) +def test_given_compile_runs_when_taking_phase_medians_then_reports_only_complete_phases( + test_case: MedianPhasesTestCase, +) -> None: + assert median_phases(runs=list(test_case.runs)) == test_case.expected_phases