From 8cbd849fc5331b3bde047a8c52b7265f7382b18d 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/9] 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 6c83a034a5bdcd71975aee537a494d9143742f3d 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/9] 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 From 33c50dc805212d3f83afc76287623d6659ca07a2 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:03:18 +0100 Subject: [PATCH 3/9] wip(native): share binding analyses across equal session members and guard model analysis CPU --- .github/workflows/verify.yml | 13 +- .../_helpers/catalog_state.rs | 21 + .../_helpers/compact_batch.rs | 468 +++++++++++++++++- .../assembly/analysis_session/constants.rs | 13 + .../_helpers/measure.py | 25 +- .../_helpers/report.py | 3 + .../compile_performance_ratio/constants.py | 1 + .../compile_performance_ratio/main/compare.py | 9 + .../compile/_helpers/assembly/project.py | 6 +- src/sqlbuild/compiler/profiling/constants.py | 1 + .../compiler/profiling/main/record_cpu.py | 24 + src/sqlbuild/compiler/profiling/types.py | 1 + .../main/build/test_compile_json_behavior.py | 1 + 13 files changed, 562 insertions(+), 24 deletions(-) create mode 100644 src/sqlbuild/compiler/profiling/main/record_cpu.py diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 28478a2601..84833fa759 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -604,6 +604,13 @@ jobs: head_engine: native-preview gate_phase: contracts_cpu_ms max_ratio: "1.00" + - kind: dense + label: native-preview model analysis vs python + base_engine: python + head_engine: native-preview + gate_phase: model_analysis_cpu_ms + compile_args: --compile-arg=--select=* + max_ratio: "1.50" steps: - uses: actions/checkout@v4 - name: Check out the comparison base @@ -630,7 +637,7 @@ 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 + - name: Compare python and native-preview phase CPU on head if: matrix.gate_phase env: KIND: ${{ matrix.kind }} @@ -638,6 +645,7 @@ jobs: BASE_ENGINE: ${{ matrix.base_engine }} HEAD_ENGINE: ${{ matrix.head_engine }} GATE_PHASE: ${{ matrix.gate_phase }} + COMPILE_ARGS: ${{ matrix.compile_args }} run: | uv run python -m scripts.compare_compile_performance \ --kind "${KIND}" \ @@ -648,7 +656,8 @@ jobs: --head-python .venv/bin/python \ --base-engine "${BASE_ENGINE}" \ --head-engine "${HEAD_ENGINE}" \ - --gate-phase "${GATE_PHASE}" + --gate-phase "${GATE_PHASE}" \ + ${COMPILE_ARGS:+"${COMPILE_ARGS}"} - name: Compare base and head cold, warm and one-model edit compiles if: ${{ !matrix.gate_phase }} env: diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs index a72ed4906b..975a6bdc4f 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs @@ -17,6 +17,8 @@ pub(crate) struct SessionCatalog { additions: Shapes, analysis_names: Vec, recorded_analysis: HashSet, + /// The `(types, nullability)` this session gave each relation's analysis. + analysis_shapes: HashMap, } impl SessionCatalog { @@ -28,6 +30,7 @@ impl SessionCatalog { additions: Vec::new(), analysis_names: Vec::new(), recorded_analysis: HashSet::new(), + analysis_shapes: HashMap::new(), } } @@ -86,6 +89,8 @@ impl SessionCatalog { name.clone(), (catalog_columns(&column_types), catalog_columns(columns)), ); + self.analysis_shapes + .insert(name.clone(), (column_types, columns.clone())); if self.recorded_analysis.insert(name.clone()) { self.analysis_names.push(name.clone()); } @@ -95,6 +100,22 @@ impl SessionCatalog { } } + /// The analysis shape this session set for `name`; inherited shapes are not known here. + pub(crate) fn session_analysis_shape(&self, name: &str) -> Option<&(Pairs, Pairs)> { + self.analysis_shapes.get(name) + } + + /// The shape a binding of `columns` for `name` resolves: Python's override or catalog entry. + pub(crate) fn effective_binding_shape(&self, name: &str, columns: &Pairs) -> Pairs { + if columns.is_empty() { + return Vec::new(); + } + match self.schemas.get(name) { + Some(known) if same_mapping(known, columns) => known.clone(), + _ => columns.clone(), + } + } + pub(crate) fn register_override(&mut self, overrides: Relations) -> usize { self.native.register_override(overrides) } diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs index 5a3b0bd450..f40a8245bf 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs @@ -1,26 +1,33 @@ -//! One compact analysis batch built, run and projected exactly as Python's unshared batch. +//! One compact analysis batch built, shared, run and projected exactly as Python's batch. use std::collections::{HashMap, HashSet}; +use std::sync::LazyLock; +use regex::{Captures, Regex}; use serde_json::{Map, Value, json}; use crate::assembly::analysis_session::_helpers::catalog_state::SessionCatalog; use crate::assembly::analysis_session::_helpers::mappings::{ShapeTable, native_dialect}; use crate::assembly::analysis_session::constants::{ BINDING_SEVERITIES, COMPACT_WORKERS, CONFIDENCE_CODES, DEFAULT_BINDING_MESSAGE, FACT_LENGTH, - INVALID_NATIVE_RESPONSE, LEGACY_FALLBACK, LEGACY_RESPONSE_LENGTH, NULLABILITY_BY_CODE, - RESPONSE_LENGTH, SOURCE_LENGTH, TRANSFORM_CODES, + INVALID_NATIVE_RESPONSE, LEGACY_FALLBACK, LEGACY_RESPONSE_LENGTH, MIN_SHARED_MEMBERS, + NULLABILITY_BY_CODE, PYTHON_ASCII_SPACES, QUALIFIED_SCAN_TOKENS, REFERENCE_MARKER_PATTERN, + RELATION_STUB_PREFIX, RESPONSE_LENGTH, SOURCE_LENGTH, TRANSFORM_CODES, + UNSTUBBED_REFERENCE_KIND, }; use crate::assembly::analysis_session::models::{ColumnFact, LineageRow}; use crate::assembly::analysis_session::types::{Pairs, Shapes}; use crate::semantic_validation::main::diagnostics::binding_diagnostics; -use crate::semantic_validation::types::{DiagnosticRow, NormalizationRequest}; +use crate::semantic_validation::types::{DiagnosticRow, NormalizationRequest, Relations}; const INVALID_BATCH: &str = "native compact query analysis returned an invalid batch response"; const INVALID_TEMPLATE: &str = "native compact query analysis returned an invalid template"; const INVALID_COLUMN: &str = "native compact analysis returned invalid column facts"; +static REFERENCE_MARKER: LazyLock> = + LazyLock::new(|| Regex::new(REFERENCE_MARKER_PATTERN).map_err(|error| error.to_string())); /// One query of a batch with the facts Python's preparation reads. +#[derive(Clone)] pub(crate) struct BatchMember<'a> { pub(crate) query_sql: &'a str, pub(crate) placeholders: &'a Pairs, @@ -73,10 +80,34 @@ struct BatchResponse<'a> { templates: &'a [Value], } -/// The prepared request and each member's query index. +/// The prepared request, each member's query index and the queries members share. struct BatchPayload { request: Value, query_indexes: Vec, + shared_queries: HashSet, +} + +/// Python's `SharedBindingQuery`: one member's relation-stubbed query and its identity. +struct SharedQuery { + sql: String, + /// Each lineage reference's stub, in reference order. + stubs: Vec<(String, String)>, + key: String, +} + +impl SharedQuery { + fn stub<'a>(&'a self, name: &'a str) -> &'a str { + self.stubs + .iter() + .find(|(reference, _)| reference == name) + .map_or(name, |(_, stub)| stub.as_str()) + } +} + +/// One run's projected members and the shared members whose result Python recomputes. +struct BatchRun { + results: Vec, + inexact: Vec, } impl SessionCatalog { @@ -98,20 +129,218 @@ impl SessionCatalog { } self.prepare_analysis(&required, batch.types, batch.nullability); let cleaned: Vec = self.normalize_members(batch)?; - let payload: BatchPayload = self.batch_payload(batch, &cleaned)?; + let shared: Vec> = self.shared_queries(batch, &cleaned)?; + let BatchRun { + mut results, + inexact, + } = self.run_batch(batch, cleaned, &shared)?; + if !inexact.is_empty() { + let subset = Batch { + members: inexact + .iter() + .map(|index| batch.members[*index].clone()) + .collect(), + ..*batch + }; + let cleaned: Vec = inexact + .iter() + .map(|index| results[*index].cleaned_sql.clone()) + .collect(); + let unshared: Vec> = inexact.iter().map(|_| None).collect(); + let rerun: BatchRun = self.run_batch(&subset, cleaned, &unshared)?; + for (index, result) in inexact.into_iter().zip(rerun.results) { + results[index] = result; + } + } + self.with_separate_validation(&batch.members, results) + } + + /// Python's `_analyze_compact_inputs` run: prepare, analyse, project and find inexact sharing. + fn run_batch( + &mut self, + batch: &Batch<'_>, + cleaned: Vec, + shared: &[Option], + ) -> Result { + let payload: BatchPayload = self.batch_payload(batch, &cleaned, shared)?; let job = self.native.prepare_compact(&payload.request.to_string())?; let analysis = job .take_analysis() .ok_or("compact analysis job already ran")?; let response: Value = serde_json::from_str(&job.run(analysis)?).map_err(|error| error.to_string())?; + let inexact: Vec = inexact_shared_members(&response, &payload); let results: Vec = attach_validations( &response, &payload.query_indexes, batch.dialect, project_response(&response, cleaned)?, )?; - self.with_separate_validation(&batch.members, results) + Ok(BatchRun { results, inexact }) + } + + /// Python's `prepare_binding_queries` shared forms, for keys at least two members match. + fn shared_queries( + &self, + batch: &Batch<'_>, + cleaned: &[String], + ) -> Result>, String> { + let prekeys: Vec> = batch + .members + .iter() + .map(|member| { + member + .binding_schema + .map(|_| share_prekey(member)) + .transpose() + }) + .collect::>()?; + let mut prekey_counts: HashMap<&str, usize> = HashMap::new(); + for prekey in prekeys.iter().flatten() { + *prekey_counts.entry(prekey.as_str()).or_default() += 1; + } + let mut candidates: Vec<(usize, Vec<(String, String)>)> = Vec::new(); + for (index, (member, prekey)) in batch.members.iter().zip(&prekeys).enumerate() { + let (Some(schema), Some(prekey)) = (member.binding_schema, prekey) else { + continue; + }; + if prekey_counts.get(prekey.as_str()).copied().unwrap_or(0) < MIN_SHARED_MEMBERS { + continue; + } + if let Some(names) = self.shared_reference_names(&cleaned[index], member, schema) { + let stubs: Vec<(String, String)> = names + .into_iter() + .enumerate() + .map(|(position, name)| (name, format!("{RELATION_STUB_PREFIX}{position}"))) + .collect(); + candidates.push((index, stubs)); + } + } + let requests: Vec = candidates + .iter() + .map(|(index, stubs)| { + let member: &BatchMember<'_> = &batch.members[*index]; + ( + member.query_sql.to_owned(), + stubs.iter().cloned().collect(), + member.placeholders.iter().cloned().collect(), + ) + }) + .collect(); + let stubbed: Vec> = if requests.is_empty() { + Vec::new() + } else { + self.native + .normalize_analysis_sqls(batch.dialect, requests)? + }; + let mut shared: Vec> = batch.members.iter().map(|_| None).collect(); + for ((index, stubs), sql) in candidates.into_iter().zip(stubbed) { + let (Ok(sql), Some(schema)) = (sql, batch.members[index].binding_schema) else { + continue; + }; + let key: String = self.shared_key( + batch, + &sql, + &stubs, + schema, + batch.members[index].recover_cte_facts, + ); + shared[index] = Some(SharedQuery { sql, stubs, key }); + } + let mut key_counts: HashMap = HashMap::new(); + for query in shared.iter().flatten() { + *key_counts.entry(query.key.clone()).or_default() += 1; + } + Ok(shared + .into_iter() + .map(|query| { + query.filter(|query| { + key_counts.get(&query.key).copied().unwrap_or(0) >= MIN_SHARED_MEMBERS + }) + }) + .collect()) + } + + /// Python's `_shared_reference_names`, None where stubbing could change the analysis. + fn shared_reference_names( + &self, + cleaned_sql: &str, + member: &BatchMember<'_>, + schema: &Shapes, + ) -> Option> { + let names: Vec = member + .lineage_references + .iter() + .map(|(name, _, _)| name.clone()) + .collect(); + let folded: HashSet = names.iter().map(|name| name.to_lowercase()).collect(); + let plain_text: bool = cleaned_sql.is_ascii() && names.iter().all(|name| name.is_ascii()); + let quoted: bool = cleaned_sql.contains('"') + || schema.iter().any(|(relation, columns)| { + relation.contains('"') || columns.iter().any(|(column, _)| column.contains('"')) + }); + let collides: bool = names + .iter() + .any(|name| name.starts_with(RELATION_STUB_PREFIX)) + || cleaned_sql + .to_ascii_lowercase() + .contains(RELATION_STUB_PREFIX); + let unshaped: bool = names + .iter() + .any(|name| self.session_analysis_shape(name).is_none()); + let usable: bool = !names.is_empty() + && folded.len() == names.len() + && plain_text + && !quoted + && !collides + && !unshaped + && !names_qualify(cleaned_sql, &names); + usable.then_some(names) + } + + /// Python's `SharedBindingQuery.key`, with the CTE fact flag its query key adds. + fn shared_key( + &self, + batch: &Batch<'_>, + sql: &str, + stubs: &[(String, String)], + schema: &Shapes, + recover_cte_facts: bool, + ) -> String { + let analysis_shapes: Vec = stubs + .iter() + .map(|(name, stub)| { + let (types, nullability) = self + .session_analysis_shape(name) + .cloned() + .unwrap_or_default(); + json!([stub, types, nullability]) + }) + .collect(); + let mut binding_shapes: Vec = schema + .iter() + .map(|(relation, columns)| { + let alias: &str = stubs + .iter() + .find(|(name, _)| name == relation) + .map_or(relation.as_str(), |(_, stub)| stub.as_str()); + json!([ + alias, + !columns.is_empty(), + self.effective_binding_shape(relation, columns) + ]) + .to_string() + }) + .collect(); + binding_shapes.sort_unstable(); + json!([ + sql, + batch.dialect, + analysis_shapes, + binding_shapes, + recover_cte_facts + ]) + .to_string() } fn normalize_members(&self, batch: &Batch<'_>) -> Result, String> { @@ -137,6 +366,7 @@ impl SessionCatalog { &mut self, batch: &Batch<'_>, cleaned: &[String], + shared: &[Option], ) -> Result { let function_return_types: Map = batch .function_return_types @@ -145,20 +375,40 @@ impl SessionCatalog { .collect(); let mut queries: Vec = Vec::new(); let mut query_keys: HashMap = HashMap::new(); + let mut shared_queries: HashSet = HashSet::new(); let mut templates: Vec = Vec::new(); let mut template_keys: HashMap = HashMap::new(); let mut projections: Vec = Vec::with_capacity(batch.members.len()); let mut query_indexes: Vec = Vec::with_capacity(batch.members.len()); - for (member, sql) in batch.members.iter().zip(cleaned) { - let query: Value = self.member_query(batch.dialect, member, sql)?; - let query_index: usize = *query_keys.entry(query.to_string()).or_insert_with(|| { - queries.push(query); - queries.len() - 1 - }); + for ((member, sql), share) in batch.members.iter().zip(cleaned).zip(shared) { + let query_index: usize = match share { + Some(share) => { + let binding = self.member_binding(member, sql)?; + let index: usize = match query_keys.get(&share.key) { + Some(index) => *index, + None => { + let query: Value = + self.shared_member_query(batch.dialect, member, share, binding); + queries.push(query); + query_keys.insert(share.key.clone(), queries.len() - 1); + queries.len() - 1 + } + }; + shared_queries.insert(index); + index + } + None => { + let query: Value = self.member_query(batch.dialect, member, sql)?; + *query_keys.entry(query.to_string()).or_insert_with(|| { + queries.push(query); + queries.len() - 1 + }) + } + }; query_indexes.push(query_index); let template: Value = json!({ "queryIndex": query_index, - "references": lineage_resources(member), + "references": lineage_resources(member, share.as_ref()), "functionReturnTypes": function_return_types, "declaredColumnOrder": declared_column_order(member, batch.nullability), "recoverCteFacts": member.recover_cte_facts, @@ -174,7 +424,12 @@ impl SessionCatalog { let resource_names: Map = member .lineage_references .iter() - .map(|(name, _, resource)| (name.clone(), Value::String(resource.clone()))) + .map(|(name, _, resource)| { + let canonical: &str = share + .as_ref() + .map_or(name.as_str(), |share| share.stub(name)); + (canonical.to_owned(), Value::String(resource.clone())) + }) .collect(); projections .push(json!({"templateIndex": template_index, "resourceNames": resource_names})); @@ -187,9 +442,64 @@ impl SessionCatalog { "workers": COMPACT_WORKERS, }), query_indexes, + shared_queries, }) } + /// A bound member's sorted binding references and overrides, preparing its relations. + fn member_binding( + &mut self, + member: &BatchMember<'_>, + sql: &str, + ) -> Result<(Vec<(String, bool)>, Relations), String> { + let schema: &Shapes = member + .binding_schema + .ok_or("a shared member has no binding schema")?; + let (_, references, overrides) = self + .prepare(&[(sql, schema)]) + .pop() + .ok_or("binding preparation returned no request")?; + Ok((references, overrides)) + } + + /// Python's shared query: stubbed SQL with references, aliases and binding sorted by stub. + fn shared_member_query( + &mut self, + dialect: &str, + member: &BatchMember<'_>, + share: &SharedQuery, + (references, overrides): (Vec<(String, bool)>, Relations), + ) -> Value { + let mut lineage_names: Vec<&str> = member + .lineage_references + .iter() + .map(|(name, _, _)| name.as_str()) + .collect(); + lineage_names.sort_by_key(|name| share.stub(name)); + let analysis_references: Vec<[&str; 2]> = lineage_names + .iter() + .map(|name| [*name, share.stub(name)]) + .collect(); + let mut references: Vec<(String, bool)> = references; + references.sort_by(|(left, _), (right, _)| share.stub(left).cmp(share.stub(right))); + let aliases: Map = references + .iter() + .filter(|(name, _)| share.stub(name) != name.as_str()) + .map(|(name, _)| (name.clone(), Value::String(share.stub(name).to_owned()))) + .collect(); + let mut query: Value = json!({ + "sql": share.sql, + "dialect": dialect, + "analysis_references": analysis_references, + "binding_references": references, + "binding_aliases": aliases, + }); + if !overrides.is_empty() { + query["binding_override"] = json!(self.register_override(overrides)); + } + query + } + /// One member's query: analysis references and, when bound, its catalog binding. fn member_query( &mut self, @@ -260,19 +570,141 @@ impl SessionCatalog { } } -fn lineage_resources(member: &BatchMember<'_>) -> Map { +fn lineage_resources(member: &BatchMember<'_>, share: Option<&SharedQuery>) -> Map { member .lineage_references .iter() .map(|(name, resource_type, _)| { + let canonical: &str = share.map_or(name.as_str(), |share| share.stub(name)); ( - name.clone(), - json!({"resourceType": resource_type, "resourceName": name}), + canonical.to_owned(), + json!({"resourceType": resource_type, "resourceName": canonical}), ) }) .collect() } +/// Python's `binding_share_prekey`: authored SQL with lineage reference markers as stubs. +fn share_prekey(member: &BatchMember<'_>) -> Result { + let marker: &Regex = REFERENCE_MARKER.as_ref().map_err(Clone::clone)?; + let positions: HashMap<&str, usize> = member + .lineage_references + .iter() + .enumerate() + .map(|(position, (name, _, _))| (name.as_str(), position)) + .collect(); + Ok(marker + .replace_all(member.query_sql, |captures: &Captures<'_>| { + let whole: &str = captures.get(0).map_or("", |found| found.as_str()); + let kind: &str = captures.get(1).map_or("", |found| found.as_str()); + match captures + .get(2) + .and_then(|found| positions.get(found.as_str())) + { + Some(position) if kind != UNSTUBBED_REFERENCE_KIND => { + format!("{RELATION_STUB_PREFIX}{position}") + } + _ => whole.to_owned(), + } + }) + .into_owned()) +} + +/// Python's `_qualified_reference_names` is non-empty; text its scanner reads counts as qualified. +fn names_qualify(sql: &str, names: &[String]) -> bool { + if !sql.contains('.') { + return false; + } + if !sql.is_ascii() + || QUALIFIED_SCAN_TOKENS + .iter() + .any(|token| sql.contains(token)) + { + return true; + } + let folded: String = sql.to_ascii_lowercase(); + names.iter().any(|name| { + let name: String = name.to_lowercase(); + plain_identifier(&name) && is_qualifier(&folded, &name) + }) +} + +/// Python's `_PLAIN_IDENTIFIER_PATTERN`. +fn plain_identifier(name: &str) -> bool { + let mut characters = name.chars(); + characters + .next() + .is_some_and(|first| first.is_ascii_lowercase() || first == '_' || first == '$') + && characters.all(|character| { + character.is_ascii_lowercase() + || character.is_ascii_digit() + || character == '_' + || character == '$' + }) +} + +/// Python's `_is_qualifier`: `name` as a whole identifier followed by `.` after any spaces. +fn is_qualifier(folded_sql: &str, name: &str) -> bool { + let identifier = |byte: u8| byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'$'; + let bytes: &[u8] = folded_sql.as_bytes(); + folded_sql.match_indices(name).any(|(start, _)| { + let mut end: usize = start + name.len(); + let bounded: bool = (start == 0 || !identifier(bytes[start - 1])) + && (end == bytes.len() || !identifier(bytes[end])); + while bounded && end < bytes.len() && PYTHON_ASCII_SPACES.as_bytes().contains(&bytes[end]) { + end += 1; + } + bounded && bytes.get(end) == Some(&b'.') + }) +} + +/// Python's `_inexact_shared_members`: shared members whose result depends on relation names. +fn inexact_shared_members(response: &Value, payload: &BatchPayload) -> Vec { + if payload.shared_queries.is_empty() { + return Vec::new(); + } + let (Some(analyses), Some(templates)) = ( + response.get("analyses").and_then(Value::as_array), + response.get("templates").and_then(Value::as_array), + ) else { + return Vec::new(); + }; + let validations: Option<&Vec> = response.get("validations").and_then(Value::as_array); + payload + .query_indexes + .iter() + .zip(analyses) + .enumerate() + .filter(|(_, (query, analysis))| { + payload.shared_queries.contains(query) && { + let template: Option<&Value> = analysis + .as_array() + .and_then(|entry| entry.first()) + .and_then(Value::as_u64) + .and_then(|index| usize::try_from(index).ok()) + .and_then(|index| templates.get(index)); + let validation: Option<&Value> = + validations.and_then(|validations| validations.get(**query)); + !shared_result_is_exact(template, validation) + } + }) + .map(|(member, _)| member) + .collect() +} + +/// Python's `shared_result_is_exact`: an analysed template without any binding diagnostic. +fn shared_result_is_exact(template: Option<&Value>, validation: Option<&Value>) -> bool { + let analysed: bool = template.is_some_and(|template| !template.is_string()); + analysed + && match validation { + None | Some(Value::Null) => true, + Some(validation) => validation + .get("errors") + .and_then(Value::as_array) + .is_some_and(Vec::is_empty), + } +} + /// A single-reference member's declared column order, as Python's template records it. fn declared_column_order<'a>( member: &BatchMember<'_>, diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/constants.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/constants.rs index d0d6fa5520..c7661e62af 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/constants.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/constants.rs @@ -95,3 +95,16 @@ pub(crate) const RENDERED_TYPE_NAMES: [(&str, &str); 12] = [ ]; /// The stack a standalone dynamic pivot proof parses on, as the analysis workers have. pub(crate) const PIVOT_WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; +/// Python's `COMPACT_RELATION_STUB_PREFIX`: the relation name a shared query binds instead. +pub(crate) const RELATION_STUB_PREFIX: &str = "__sqlbuild_project_input_"; +/// Python's `COMPACT_REFERENCE_MARKER_PATTERN`: a reference call and the name it reads. +pub(crate) const REFERENCE_MARKER_PATTERN: &str = + r#"__(ref|seed|source|dbt_ref)\((?:"[^"]+"\s*,\s*)?"([^"]+)"\)"#; +/// Python's `COMPACT_UNSTUBBED_REFERENCE_KIND`, a reference kind sharing never stubs. +pub(crate) const UNSTUBBED_REFERENCE_KIND: &str = "dbt_ref"; +/// Python's `MIN_SHARED_BINDING_QUERY_MEMBERS`. +pub(crate) const MIN_SHARED_MEMBERS: usize = 2; +/// Text whose presence sends Python's qualifier search to its token scanner. +pub(crate) const QUALIFIED_SCAN_TOKENS: [&str; 5] = ["\"", "`", "[", "--", "/*"]; +/// Python's `str.isspace` over ASCII. +pub(crate) const PYTHON_ASCII_SPACES: &str = " \t\n\r\x0b\x0c\x1c\x1d\x1e\x1f"; diff --git a/scripts/compile_performance_ratio/_helpers/measure.py b/scripts/compile_performance_ratio/_helpers/measure.py index 0f3882c47a..27a314ad22 100644 --- a/scripts/compile_performance_ratio/_helpers/measure.py +++ b/scripts/compile_performance_ratio/_helpers/measure.py @@ -118,6 +118,7 @@ def compare_builds( runs: int, modes: tuple[str, ...], engines: tuple[str | None, str | None] = (None, None), + compile_args: tuple[str, ...] = (), ) -> tuple[CompileComparison, ...]: """Alternate base and head compiles per mode; warm and edit reuse the cold project.""" @@ -133,7 +134,12 @@ def compare_builds( if mode == COLD_MODE: for label, python, project_dir, engine in builds: _ = _compile_once( - label=label, python=python, project_dir=project_dir, mode=mode, engine=engine + label=label, + python=python, + project_dir=project_dir, + mode=mode, + engine=engine, + compile_args=compile_args, ) cache_primed = False elif not cache_primed: @@ -144,6 +150,7 @@ def compare_builds( project_dir=project_dir, mode=WARM_MODE, engine=engine, + compile_args=compile_args, ) cache_primed = True results: dict[str, list[CompileRun]] = {BASE_LABEL: [], HEAD_LABEL: []} @@ -152,7 +159,12 @@ def compare_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, engine=engine + label=label, + python=python, + project_dir=project_dir, + mode=mode, + engine=engine, + compile_args=compile_args, ) _check_cache_use(run=run, mode=mode) results[label].append(run) @@ -201,7 +213,13 @@ def _check_cache_use(*, run: CompileRun, mode: str) -> None: def _compile_once( - *, label: str, python: Path, project_dir: Path, mode: str, engine: str | None + *, + label: str, + python: Path, + project_dir: Path, + mode: str, + engine: str | None, + compile_args: tuple[str, ...], ) -> CompileRun: cache_args: tuple[str, ...] = () if mode == COLD_MODE: @@ -223,6 +241,7 @@ def _compile_once( "compile", "--json", *cache_args, + *compile_args, ], capture_output=True, text=True, diff --git a/scripts/compile_performance_ratio/_helpers/report.py b/scripts/compile_performance_ratio/_helpers/report.py index 2cc764acf3..aabf811eea 100644 --- a/scripts/compile_performance_ratio/_helpers/report.py +++ b/scripts/compile_performance_ratio/_helpers/report.py @@ -20,6 +20,7 @@ def comparison_markdown( mode_max_ratios: Mapping[str, float] | None = None, engines: tuple[str | None, str | None] = (None, None), gate_phases: tuple[str, ...] = (), + compile_args: tuple[str, ...] = (), ) -> str: """Summarize medians, ratios and per-phase timings of every mode as Markdown tables.""" @@ -39,6 +40,8 @@ def comparison_markdown( ) if gate_phases: lines.extend((f"Gated phases: {', '.join(gate_phases)}.", "")) + if compile_args: + lines.extend((f"Compile arguments: `{' '.join(compile_args)}`.", "")) for comparison in comparisons: lines.extend( ( diff --git a/scripts/compile_performance_ratio/constants.py b/scripts/compile_performance_ratio/constants.py index d51fa2cd83..0a365a0367 100644 --- a/scripts/compile_performance_ratio/constants.py +++ b/scripts/compile_performance_ratio/constants.py @@ -47,6 +47,7 @@ "total_ms", "graph_ms", "model_analysis_ms", + "model_analysis_cpu_ms", "analysis_native_ms", "contracts_ms", "contracts_cpu_ms", diff --git a/scripts/compile_performance_ratio/main/compare.py b/scripts/compile_performance_ratio/main/compare.py index 322780b793..cbf0cbbe29 100644 --- a/scripts/compile_performance_ratio/main/compare.py +++ b/scripts/compile_performance_ratio/main/compare.py @@ -62,6 +62,7 @@ def compare_compile_performance(argv: list[str] | None = None) -> int: runs=args.runs, modes=modes, engines=(args.base_engine, args.head_engine), + compile_args=tuple(args.compile_args), ) mode_max_ratios: dict[str, float] = ( {} if args.edit_max_ratio is None else {EDIT_MODE: args.edit_max_ratio} @@ -93,6 +94,7 @@ def compare_compile_performance(argv: list[str] | None = None) -> int: mode_max_ratios=mode_max_ratios, engines=(args.base_engine, args.head_engine), gate_phases=gate_phases, + compile_args=tuple(args.compile_args), ) print(markdown) _ = append_summary(path=Path(summary_value) if summary_value else None, markdown=markdown) @@ -148,6 +150,13 @@ def _parse_args(argv: list[str] | None) -> argparse.Namespace: default=None, help="Compiler engine the head build runs; defaults to the build's own default.", ) + parser.add_argument( + "--compile-arg", + dest="compile_args", + action="append", + default=[], + help="Extra `sqb compile` argument for both builds, e.g. `--compile-arg=--select=*`.", + ) parser.add_argument( "--gate-phase", dest="gate_phases", diff --git a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py index 87bf84e5ab..119e062eeb 100644 --- a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py +++ b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py @@ -166,6 +166,7 @@ from sqlbuild.compiler.lineage.types import ColumnLineageMode, InferredNullability from sqlbuild.compiler.planner.types import ContractPolicy from sqlbuild.compiler.profiling.main.record import record_compile_timing +from sqlbuild.compiler.profiling.main.record_cpu import record_compile_cpu_timing from sqlbuild.compiler.project_assembly.models import NativeProjectResources from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.resource_names.main.function_node_type import function_node_type @@ -326,7 +327,10 @@ def assemble_compiled_project( ) model_sql_analysis_by_name: dict[str, _ModelSqlAnalysis] = {} if sql_analysis_enabled: - with record_compile_timing("model_analysis_ms"): + with ( + record_compile_timing("model_analysis_ms"), + record_compile_cpu_timing("model_analysis_cpu_ms"), + ): python_analysis: partial[dict[str, _ModelSqlAnalysis]] = partial( _analyze_model_sql_in_parallel, known_functions=known_function_names(inputs.sql_function_inputs), diff --git a/src/sqlbuild/compiler/profiling/constants.py b/src/sqlbuild/compiler/profiling/constants.py index 1ff96dd869..7cf606a237 100644 --- a/src/sqlbuild/compiler/profiling/constants.py +++ b/src/sqlbuild/compiler/profiling/constants.py @@ -5,6 +5,7 @@ COMPILE_TIMING_PHASES: tuple[CompileTimingPhase, ...] = ( "attachment_ms", "model_analysis_ms", + "model_analysis_cpu_ms", "analysis_preparation_ms", "analysis_native_ms", "analysis_projection_ms", diff --git a/src/sqlbuild/compiler/profiling/main/record_cpu.py b/src/sqlbuild/compiler/profiling/main/record_cpu.py new file mode 100644 index 0000000000..18b1ab7aba --- /dev/null +++ b/src/sqlbuild/compiler/profiling/main/record_cpu.py @@ -0,0 +1,24 @@ +"""Compile phase CPU recording entry point.""" + +import time +from collections.abc import Iterator +from contextlib import contextmanager + +from sqlbuild.compiler.profiling.classes.context import CompileTimingContext +from sqlbuild.compiler.profiling.models import CompileTimingCollector +from sqlbuild.compiler.profiling.types import CompileTimingPhase + + +@contextmanager +def record_compile_cpu_timing(phase: CompileTimingPhase) -> Iterator[None]: + """Record the process CPU time an operation spans, including its worker threads.""" + + collector: CompileTimingCollector | None = CompileTimingContext.active.get() + if collector is None: + yield + return + start_ns: int = time.process_time_ns() + try: + yield + finally: + collector.add(phase=phase, elapsed_ns=time.process_time_ns() - start_ns) diff --git a/src/sqlbuild/compiler/profiling/types.py b/src/sqlbuild/compiler/profiling/types.py index 22f51e54cc..59ff6ef9d8 100644 --- a/src/sqlbuild/compiler/profiling/types.py +++ b/src/sqlbuild/compiler/profiling/types.py @@ -5,6 +5,7 @@ type CompileTimingPhase = Literal[ "attachment_ms", "model_analysis_ms", + "model_analysis_cpu_ms", "analysis_preparation_ms", "analysis_native_ms", "analysis_projection_ms", 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 dff9027895..c7b7680d7c 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 @@ -73,6 +73,7 @@ def test_given_waffle_shop_when_running_compile_json_then_it_reports_offline_que "custom_rules_ms", "early_lint_wait_ms", "model_analysis_ms", + "model_analysis_cpu_ms", "analysis_preparation_ms", "analysis_native_ms", "analysis_projection_ms", From 5cec154ae1e949688586506ad4a69b6e54aa5870 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:12:28 +0100 Subject: [PATCH 4/9] test(native): pin shared, re-analysed and unshared session members against Python --- .../_helpers/catalog_state.rs | 9 +++ .../_helpers/compact_batch.rs | 2 + .../src/assembly/analysis_session/main/mod.rs | 1 + .../analysis_session/main/session_sharing.rs | 8 +++ .../_helpers/analysis_session/session.rs | 10 ++++ src/sqlbuild/_native.pyi | 2 + .../compiler/analysis_session/_test_types.py | 14 +++++ .../compiler/analysis_session/helpers.py | 54 ++++++++++++++++++ .../test_native_model_analysis.py | 56 +++++++++++++++++++ 9 files changed, 156 insertions(+) create mode 100644 crates/sqlbuild-analysis/src/assembly/analysis_session/main/session_sharing.rs diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs index 975a6bdc4f..0bbe164e2e 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/catalog_state.rs @@ -19,6 +19,8 @@ pub(crate) struct SessionCatalog { recorded_analysis: HashSet, /// The `(types, nullability)` this session gave each relation's analysis. analysis_shapes: HashMap, + pub(crate) shared_members: usize, + pub(crate) reanalysed_members: usize, } impl SessionCatalog { @@ -31,6 +33,8 @@ impl SessionCatalog { analysis_names: Vec::new(), recorded_analysis: HashSet::new(), analysis_shapes: HashMap::new(), + shared_members: 0, + reanalysed_members: 0, } } @@ -100,6 +104,11 @@ impl SessionCatalog { } } + /// `(shared members, shared members re-analysed alone)` so far. + pub(crate) fn sharing(&self) -> (usize, usize) { + (self.shared_members, self.reanalysed_members) + } + /// The analysis shape this session set for `name`; inherited shapes are not known here. pub(crate) fn session_analysis_shape(&self, name: &str) -> Option<&(Pairs, Pairs)> { self.analysis_shapes.get(name) diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs index f40a8245bf..eb7b4b8365 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs @@ -134,6 +134,8 @@ impl SessionCatalog { mut results, inexact, } = self.run_batch(batch, cleaned, &shared)?; + self.shared_members += shared.iter().flatten().count(); + self.reanalysed_members += inexact.len(); if !inexact.is_empty() { let subset = Batch { members: inexact diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs index 6c5394355c..5c08bc3722 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs @@ -3,4 +3,5 @@ pub mod finish_analysis_session; pub mod prove_dynamic_contract; pub mod provide_deferred_analyses; pub mod run_analysis_session; +pub mod session_sharing; pub mod start_analysis_session; diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/session_sharing.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/session_sharing.rs new file mode 100644 index 0000000000..3b67b99626 --- /dev/null +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/session_sharing.rs @@ -0,0 +1,8 @@ +//! How many of a session's analysed models shared a query, and how many it re-analysed. + +use crate::assembly::analysis_session::models::AnalysisSession; + +/// `(shared members, shared members re-analysed alone)` across the session's batches so far. +pub fn session_sharing(session: &AnalysisSession) -> (usize, usize) { + session.catalog.sharing() +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs b/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs index 87af8f9b33..374e2bc188 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs @@ -7,6 +7,7 @@ use sqlbuild_analysis::assembly::analysis_session::main::finish_analysis_session use sqlbuild_analysis::assembly::analysis_session::main::prove_dynamic_contract::prove_dynamic_contract; use sqlbuild_analysis::assembly::analysis_session::main::provide_deferred_analyses::provide_deferred_analyses; use sqlbuild_analysis::assembly::analysis_session::main::run_analysis_session::run_analysis_session; +use sqlbuild_analysis::assembly::analysis_session::main::session_sharing::session_sharing; use sqlbuild_analysis::assembly::analysis_session::main::start_analysis_session::start_analysis_session; use sqlbuild_analysis::assembly::analysis_session::models::{ AnalysisSession, ColumnFact, Deferral, DeferredAnalysis, DynamicFamily, ExpressionShape, @@ -117,6 +118,7 @@ type FinishRow = (Vec, Shapes, Vec, Vec); pub(crate) struct NativeModelAnalysisSession { inner: Option, failure: Option, + sharing: (usize, usize), } impl NativeModelAnalysisSession { @@ -143,6 +145,7 @@ impl NativeModelAnalysisSession { run_analysis_session(&mut session).map(|step| (session, step)) }); let (session, step) = self.kept(result)?; + self.sharing = session_sharing(&session); self.inner = Some(session); Some(step_row(step)) } @@ -172,6 +175,12 @@ impl NativeModelAnalysisSession { fn failure(&self) -> Option { self.failure.clone() } + + /// `(shared members, shared members re-analysed alone)` as of the last step. + #[getter] + fn sharing(&self) -> (usize, usize) { + self.sharing + } } /// Start a session on `catalog`, or None where Python must analyse. @@ -189,6 +198,7 @@ fn start_model_analysis_session( Ok(session.map(|session| NativeModelAnalysisSession { inner: Some(session), failure: None, + sharing: (0, 0), })) } diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 5d194a0bf1..ccd6fb79b2 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -422,6 +422,8 @@ class NativeModelAnalysisSession: ): ... @property def failure(self) -> str | None: ... + @property + def sharing(self) -> tuple[int, int]: ... def start_model_analysis_session( catalog: object, request: tuple[object, ...], / diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py b/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py index f3ea3a0aac..9ce474e84b 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py @@ -49,3 +49,17 @@ class StandalonePivotProofTestCase: analysed_models: frozenset[str] expected_native_proofs: int expected_proven_by_model: dict[str, bool | None] + + +@dataclass(frozen=True) +class SharedAnalysisTestCase: + """Models whose equal queries share one analysis, some inexact and so re-analysed alone.""" + + description: str + regions: tuple[str, ...] + inexact_regions: tuple[str, ...] + dialects: tuple[str | None, ...] + expected_analysed: int + expected_shared: int + expected_reanalysed: int + expected_unshared: int diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py index 47095774c0..8115e45b60 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py @@ -349,6 +349,60 @@ def generated_analysis_files(*, rng: random.Random, model_count: int) -> dict[st return files +def shared_analysis_files( + *, regions: tuple[str, ...], inexact_regions: tuple[str, ...] +) -> dict[str, str]: + """Equal regional models and rollups, missing-column readers and one unshared summary.""" + + regional: dict[str, str] = { + f"models/staging/orders_{region}.sql": ( + f'MODEL (description "Orders in {region}");\n\n' + 'SELECT order_id, customer_id, amount FROM __source("raw_orders")\n' + ) + for region in regions + } + rollups: dict[str, str] = { + f"models/marts/totals_{region}.sql": ( + f'MODEL (description "Customer totals in {region}");\n\n' + "SELECT customer_id, SUM(amount) AS total_amount\n" + f'FROM __ref("orders_{region}")\nGROUP BY customer_id\n' + ) + for region in regions + } + inexact: dict[str, str] = { + f"models/marts/missing_{region}.sql": ( + f'MODEL (description "Orders in {region} with an unknown column");\n\n' + f'SELECT customer_id, missing_column FROM __ref("orders_{region}")\n' + ) + for region in inexact_regions + } + return { + "sqlbuild_project.toml": _PROJECT_TOML, + "sources/raw.yml": _SOURCES, + **regional, + **rollups, + **inexact, + "models/marts/orders_summary.sql": ( + 'MODEL (description "Order count");\n\n' + f'SELECT COUNT(*) AS order_count FROM __ref("orders_{regions[0]}")\n' + ), + } + + +def started_sessions(*, monkeypatch: pytest.MonkeyPatch) -> list[Any]: + """Record every native session started; return the list they are appended to.""" + + started: list[Any] = [] + original: Callable[..., Any] = native_module.start_model_analysis_session + + def start(catalog: object, request: tuple[object, ...]) -> Any: + started.append(original(catalog, request)) + return started[-1] + + monkeypatch.setattr(native_module, "start_model_analysis_session", start) + return started + + def compile_inputs(*, project_dir: Path, files: dict[str, str]) -> CompileProjectInputs: """Write a project and attach its compile inputs with the DuckDB compile context.""" diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py index 4e9527ea64..37ca83b82c 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py @@ -27,6 +27,7 @@ AnalysisFallbackTestCase, GeneratedAnalysisParityTestCase, SessionFailureTestCase, + SharedAnalysisTestCase, StandalonePivotProofTestCase, ) from tests.integration.src.sqlbuild.compiler.analysis_session.helpers import ( @@ -40,6 +41,8 @@ generated_analysis_files, native_pivot_proofs, pivot_project_files, + shared_analysis_files, + started_sessions, ) from tests.integration.src.sqlbuild.compiler.helpers import mismatches @@ -105,6 +108,59 @@ def test_given_generated_projects_when_analysing_natively_then_matches_python( assert parity.native_enrichments >= test_case.expected_minimum_native_enrichments +@pytest.mark.parametrize( + "test_case", + [ + SharedAnalysisTestCase( + description="equal regional queries, missing-column readers and a unique summary", + regions=("east", "west", "north", "south"), + inexact_regions=("east", "west", "north"), + dialects=("duckdb", "snowflake"), + expected_analysed=24, + expected_shared=22, + expected_reanalysed=6, + expected_unshared=2, + ) + ], + ids=lambda case: case.description, +) +def test_given_equal_model_queries_when_analysing_natively_then_shares_and_matches_python( + test_case: SharedAnalysisTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + record_dir: Path = tmp_path / "records" + monkeypatch.setenv(ANALYSIS_RECORD_DIR_ENV_VAR, str(record_dir)) + inputs: CompileProjectInputs = compile_inputs( + project_dir=tmp_path / "project", + files=shared_analysis_files( + regions=test_case.regions, inexact_regions=test_case.inexact_regions + ), + ) + sessions: list[object] = started_sessions(monkeypatch=monkeypatch) + parity: AnalysisParity = AnalysisParity() + + _ = [ + compare_analyses(inputs=inputs, dialect=dialect, parity=parity, monkeypatch=monkeypatch) + for dialect in test_case.dialects + ] + + shared: int = sum(getattr(session, "sharing")[0] for session in sessions) + + assert mismatches(inputs=parity.names, expected=parity.python, actual=parity.native) == [] + assert ( + parity.analysed_models, + shared, + sum(getattr(session, "sharing")[1] for session in sessions), + parity.analysed_models - shared, + deferral_kinds(record_dir), + ) == ( + test_case.expected_analysed, + test_case.expected_shared, + test_case.expected_reanalysed, + test_case.expected_unshared, + Counter(), + ) + + @pytest.mark.parametrize( "test_case", [ From e4da4130f76562373cf8b90e0a4caf6b80aa726b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:28:01 +0100 Subject: [PATCH 5/9] test(perf): cover the model analysis CPU gate with compile arguments --- .../compile_performance_ratio/_test_types.py | 2 ++ .../test_compare_compile_performance.py | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/e2e/scripts/compile_performance_ratio/_test_types.py b/tests/e2e/scripts/compile_performance_ratio/_test_types.py index 4df5ed04c2..dec1ba1132 100644 --- a/tests/e2e/scripts/compile_performance_ratio/_test_types.py +++ b/tests/e2e/scripts/compile_performance_ratio/_test_types.py @@ -32,3 +32,5 @@ class EnginePhaseRatioTestCase: phase_noise_floor_ms: str expected_return_code: int expected_fragments: tuple[str, ...] + gate_phase: str = "contracts_cpu_ms" + compile_args: 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 cfda3aaa37..e85352ce58 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 @@ -193,6 +193,19 @@ def test_given_base_root_when_comparing_compile_performance_then_each_side_uses_ "**Result: failed.**", ), ), + EnginePhaseRatioTestCase( + description="model analysis CPU over its allowance with rules off fails on it alone", + max_ratio="0", + phase_noise_floor_ms="-1", + expected_return_code=1, + expected_fragments=( + "Compile arguments: `--select=*`.", + "| model_analysis_cpu_ms |", + "Compile performance regression: dense 20 cold model_analysis_cpu_ms:", + ), + gate_phase="model_analysis_cpu_ms", + compile_args=("--compile-arg=--select=*",), + ), ), ids=lambda case: case.description, ) @@ -223,11 +236,12 @@ def test_given_two_engines_when_gating_a_phase_then_enforces_only_that_phase( "--head-engine", "native-preview", "--gate-phase", - "contracts_cpu_ms", + test_case.gate_phase, "--max-ratio", test_case.max_ratio, "--phase-noise-floor-ms", test_case.phase_noise_floor_ms, + *test_case.compile_args, ], cwd=_REPO_ROOT, capture_output=True, From 8e8de9104989c8095a9c6796aae4cabf6bc9c4d9 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:59:20 +0100 Subject: [PATCH 6/9] refactor(native): name the sharing helpers fensu requires and keep the CPU timer domain-private --- .../_helpers/compact_batch.rs | 98 ++++++++++++------- .../compile/_helpers/assembly/project.py | 2 +- .../main/{record_cpu.py => _record_cpu.py} | 0 .../test_native_model_analysis.py | 7 +- 4 files changed, 67 insertions(+), 40 deletions(-) rename src/sqlbuild/compiler/profiling/main/{record_cpu.py => _record_cpu.py} (100%) diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs index eb7b4b8365..4b5186a968 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/compact_batch.rs @@ -97,10 +97,7 @@ struct SharedQuery { impl SharedQuery { fn stub<'a>(&'a self, name: &'a str) -> &'a str { - self.stubs - .iter() - .find(|(reference, _)| reference == name) - .map_or(name, |(_, stub)| stub.as_str()) + stub_name(&self.stubs, name) } } @@ -190,12 +187,7 @@ impl SessionCatalog { let prekeys: Vec> = batch .members .iter() - .map(|member| { - member - .binding_schema - .map(|_| share_prekey(member)) - .transpose() - }) + .map(member_prekey) .collect::>()?; let mut prekey_counts: HashMap<&str, usize> = HashMap::new(); for prekey in prekeys.iter().flatten() { @@ -241,11 +233,10 @@ impl SessionCatalog { continue; }; let key: String = self.shared_key( - batch, + batch.dialect, &sql, &stubs, - schema, - batch.members[index].recover_cte_facts, + (schema, batch.members[index].recover_cte_facts), ); shared[index] = Some(SharedQuery { sql, stubs, key }); } @@ -255,11 +246,7 @@ impl SessionCatalog { } Ok(shared .into_iter() - .map(|query| { - query.filter(|query| { - key_counts.get(&query.key).copied().unwrap_or(0) >= MIN_SHARED_MEMBERS - }) - }) + .map(|query| with_partner(query, &key_counts)) .collect()) } @@ -277,10 +264,7 @@ impl SessionCatalog { .collect(); let folded: HashSet = names.iter().map(|name| name.to_lowercase()).collect(); let plain_text: bool = cleaned_sql.is_ascii() && names.iter().all(|name| name.is_ascii()); - let quoted: bool = cleaned_sql.contains('"') - || schema.iter().any(|(relation, columns)| { - relation.contains('"') || columns.iter().any(|(column, _)| column.contains('"')) - }); + let quoted: bool = cleaned_sql.contains('"') || schema_quoted(schema); let collides: bool = names .iter() .any(|name| name.starts_with(RELATION_STUB_PREFIX)) @@ -303,11 +287,10 @@ impl SessionCatalog { /// Python's `SharedBindingQuery.key`, with the CTE fact flag its query key adds. fn shared_key( &self, - batch: &Batch<'_>, + dialect: &str, sql: &str, stubs: &[(String, String)], - schema: &Shapes, - recover_cte_facts: bool, + (schema, recover_cte_facts): (&Shapes, bool), ) -> String { let analysis_shapes: Vec = stubs .iter() @@ -322,10 +305,7 @@ impl SessionCatalog { let mut binding_shapes: Vec = schema .iter() .map(|(relation, columns)| { - let alias: &str = stubs - .iter() - .find(|(name, _)| name == relation) - .map_or(relation.as_str(), |(_, stub)| stub.as_str()); + let alias: &str = stub_name(stubs, relation); json!([ alias, !columns.is_empty(), @@ -337,7 +317,7 @@ impl SessionCatalog { binding_shapes.sort_unstable(); json!([ sql, - batch.dialect, + dialect, analysis_shapes, binding_shapes, recover_cte_facts @@ -679,12 +659,7 @@ fn inexact_shared_members(response: &Value, payload: &BatchPayload) -> Vec = analysis - .as_array() - .and_then(|entry| entry.first()) - .and_then(Value::as_u64) - .and_then(|index| usize::try_from(index).ok()) - .and_then(|index| templates.get(index)); + let template: Option<&Value> = analysis_template(analysis, templates); let validation: Option<&Value> = validations.and_then(|validations| validations.get(**query)); !shared_result_is_exact(template, validation) @@ -694,6 +669,57 @@ fn inexact_shared_members(response: &Value, payload: &BatchPayload) -> Vec(analysis: &Value, templates: &'a [Value]) -> Option<&'a Value> { + let index: u64 = analysis.as_array()?.first()?.as_u64()?; + match usize::try_from(index) { + Ok(index) => templates.get(index), + Err(_) => None, + } +} + +/// `stubs`' stub for `name`, or `name` itself when it has none. +fn stub_name<'a>(stubs: &'a [(String, String)], name: &'a str) -> &'a str { + for (reference, stub) in stubs { + if reference == name { + return stub; + } + } + name +} + +/// A bound member's prekey; members without a binding schema never share. +fn member_prekey(member: &BatchMember<'_>) -> Result, String> { + match member.binding_schema { + Some(_) => share_prekey(member).map(Some), + None => Ok(None), + } +} + +/// `query` when another member shares its key. +fn with_partner( + query: Option, + counts: &HashMap, +) -> Option { + let query: SharedQuery = query?; + (counts.get(&query.key).copied().unwrap_or(0) >= MIN_SHARED_MEMBERS).then_some(query) +} + +/// Whether a binding relation or column name carries a double quote. +fn schema_quoted(schema: &Shapes) -> bool { + for (relation, columns) in schema { + if relation.contains('"') { + return true; + } + for (column, _) in columns { + if column.contains('"') { + return true; + } + } + } + false +} + /// Python's `shared_result_is_exact`: an analysed template without any binding diagnostic. fn shared_result_is_exact(template: Option<&Value>, validation: Option<&Value>) -> bool { let analysed: bool = template.is_some_and(|template| !template.is_string()); diff --git a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py index 119e062eeb..d2d769c6cc 100644 --- a/src/sqlbuild/compiler/compile/_helpers/assembly/project.py +++ b/src/sqlbuild/compiler/compile/_helpers/assembly/project.py @@ -165,8 +165,8 @@ ) from sqlbuild.compiler.lineage.types import ColumnLineageMode, InferredNullability from sqlbuild.compiler.planner.types import ContractPolicy +from sqlbuild.compiler.profiling.main._record_cpu import record_compile_cpu_timing from sqlbuild.compiler.profiling.main.record import record_compile_timing -from sqlbuild.compiler.profiling.main.record_cpu import record_compile_cpu_timing from sqlbuild.compiler.project_assembly.models import NativeProjectResources from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.resource_names.main.function_node_type import function_node_type diff --git a/src/sqlbuild/compiler/profiling/main/record_cpu.py b/src/sqlbuild/compiler/profiling/main/_record_cpu.py similarity index 100% rename from src/sqlbuild/compiler/profiling/main/record_cpu.py rename to src/sqlbuild/compiler/profiling/main/_record_cpu.py diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py index 37ca83b82c..34e3dfddb0 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py @@ -6,6 +6,7 @@ from collections import Counter from dataclasses import replace from pathlib import Path +from typing import Any import pytest @@ -135,7 +136,7 @@ def test_given_equal_model_queries_when_analysing_natively_then_shares_and_match regions=test_case.regions, inexact_regions=test_case.inexact_regions ), ) - sessions: list[object] = started_sessions(monkeypatch=monkeypatch) + sessions: list[Any] = started_sessions(monkeypatch=monkeypatch) parity: AnalysisParity = AnalysisParity() _ = [ @@ -143,13 +144,13 @@ def test_given_equal_model_queries_when_analysing_natively_then_shares_and_match for dialect in test_case.dialects ] - shared: int = sum(getattr(session, "sharing")[0] for session in sessions) + shared: int = sum(session.sharing[0] for session in sessions) assert mismatches(inputs=parity.names, expected=parity.python, actual=parity.native) == [] assert ( parity.analysed_models, shared, - sum(getattr(session, "sharing")[1] for session in sessions), + sum(session.sharing[1] for session in sessions), parity.analysed_models - shared, deferral_kinds(record_dir), ) == ( From 0bca3af90b5fd9996633e13a70bd18577a9dd1b1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:02:09 +0100 Subject: [PATCH 7/9] refactor(perf): share one clock-driven timer between wall and CPU compile phases --- .../compiler/profiling/_helpers/__init__.py | 1 + .../compiler/profiling/_helpers/timing.py | 23 +++++++++++++++++++ .../compiler/profiling/main/_record_cpu.py | 19 ++++----------- .../compiler/profiling/main/record.py | 19 ++++----------- 4 files changed, 32 insertions(+), 30 deletions(-) create mode 100644 src/sqlbuild/compiler/profiling/_helpers/__init__.py create mode 100644 src/sqlbuild/compiler/profiling/_helpers/timing.py diff --git a/src/sqlbuild/compiler/profiling/_helpers/__init__.py b/src/sqlbuild/compiler/profiling/_helpers/__init__.py new file mode 100644 index 0000000000..64fb62aa62 --- /dev/null +++ b/src/sqlbuild/compiler/profiling/_helpers/__init__.py @@ -0,0 +1 @@ +"""Native model analysis session helpers.""" diff --git a/src/sqlbuild/compiler/profiling/_helpers/timing.py b/src/sqlbuild/compiler/profiling/_helpers/timing.py new file mode 100644 index 0000000000..42ccefd697 --- /dev/null +++ b/src/sqlbuild/compiler/profiling/_helpers/timing.py @@ -0,0 +1,23 @@ +"""Compile phase timing by a chosen clock.""" + +from collections.abc import Callable, Iterator +from contextlib import contextmanager + +from sqlbuild.compiler.profiling.classes.context import CompileTimingContext +from sqlbuild.compiler.profiling.models import CompileTimingCollector +from sqlbuild.compiler.profiling.types import CompileTimingPhase + + +@contextmanager +def recorded_timing(*, phase: CompileTimingPhase, clock: Callable[[], int]) -> Iterator[None]: + """Record an operation by `clock` when a compile timing collector is active.""" + + collector: CompileTimingCollector | None = CompileTimingContext.active.get() + if collector is None: + yield + return + start_ns: int = clock() + try: + yield + finally: + collector.add(phase=phase, elapsed_ns=clock() - start_ns) diff --git a/src/sqlbuild/compiler/profiling/main/_record_cpu.py b/src/sqlbuild/compiler/profiling/main/_record_cpu.py index 18b1ab7aba..a0a15e0ef7 100644 --- a/src/sqlbuild/compiler/profiling/main/_record_cpu.py +++ b/src/sqlbuild/compiler/profiling/main/_record_cpu.py @@ -1,24 +1,13 @@ """Compile phase CPU recording entry point.""" import time -from collections.abc import Iterator -from contextlib import contextmanager +from contextlib import AbstractContextManager -from sqlbuild.compiler.profiling.classes.context import CompileTimingContext -from sqlbuild.compiler.profiling.models import CompileTimingCollector +from sqlbuild.compiler.profiling._helpers.timing import recorded_timing from sqlbuild.compiler.profiling.types import CompileTimingPhase -@contextmanager -def record_compile_cpu_timing(phase: CompileTimingPhase) -> Iterator[None]: +def record_compile_cpu_timing(phase: CompileTimingPhase) -> AbstractContextManager[None]: """Record the process CPU time an operation spans, including its worker threads.""" - collector: CompileTimingCollector | None = CompileTimingContext.active.get() - if collector is None: - yield - return - start_ns: int = time.process_time_ns() - try: - yield - finally: - collector.add(phase=phase, elapsed_ns=time.process_time_ns() - start_ns) + return recorded_timing(phase=phase, clock=time.process_time_ns) diff --git a/src/sqlbuild/compiler/profiling/main/record.py b/src/sqlbuild/compiler/profiling/main/record.py index 5518e5a413..4053321d80 100644 --- a/src/sqlbuild/compiler/profiling/main/record.py +++ b/src/sqlbuild/compiler/profiling/main/record.py @@ -1,24 +1,13 @@ """Compile phase recording entry point.""" import time -from collections.abc import Iterator -from contextlib import contextmanager +from contextlib import AbstractContextManager -from sqlbuild.compiler.profiling.classes.context import CompileTimingContext -from sqlbuild.compiler.profiling.models import CompileTimingCollector +from sqlbuild.compiler.profiling._helpers.timing import recorded_timing from sqlbuild.compiler.profiling.types import CompileTimingPhase -@contextmanager -def record_compile_timing(phase: CompileTimingPhase) -> Iterator[None]: +def record_compile_timing(phase: CompileTimingPhase) -> AbstractContextManager[None]: """Record an operation when a compile timing collector is active.""" - collector: CompileTimingCollector | None = CompileTimingContext.active.get() - if collector is None: - yield - return - start_ns: int = time.perf_counter_ns() - try: - yield - finally: - collector.add(phase=phase, elapsed_ns=time.perf_counter_ns() - start_ns) + return recorded_timing(phase=phase, clock=time.perf_counter_ns) From fb606bd2404a161af429f28136934896a2b0ad79 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:34:50 +0100 Subject: [PATCH 8/9] test(native): pin that inputs differing only in nullability never share an analysis --- .../compiler/analysis_session/_test_types.py | 3 +- .../compiler/analysis_session/helpers.py | 46 +++++++++++++++++++ .../test_native_model_analysis.py | 23 +++++++--- 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py b/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py index 9ce474e84b..63d4a0b72b 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/_test_types.py @@ -56,8 +56,7 @@ class SharedAnalysisTestCase: """Models whose equal queries share one analysis, some inexact and so re-analysed alone.""" description: str - regions: tuple[str, ...] - inexact_regions: tuple[str, ...] + files: dict[str, str] dialects: tuple[str | None, ...] expected_analysed: int expected_shared: int diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py index 8115e45b60..645bb8944f 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py @@ -389,6 +389,52 @@ def shared_analysis_files( } +def nullability_analysis_files() -> dict[str, str]: + """Equal queries over inputs whose types match but whose nullability may differ.""" + + sources: str = "sources:\n" + "".join( + f" - name: {name}\n description: Orders.\n columns:\n" + " - name: customer_id\n type: INTEGER\n" + f" - name: amount\n type: DOUBLE{nullable}\n" + for name, nullable in ( + ("s1", ""), + ("s2", "\n nullable: false"), + ("s3", "\n nullable: true"), + ) + ) + readers: dict[str, str] = { + f"models/m_{name}.sql": ( + 'MODEL (description "Orders read with null handling");\n\n' + "SELECT customer_id, amount, COALESCE(amount, 0) AS filled, amount IS NULL AS missing,\n" + f' amount + 1 AS bumped\nFROM __source("{name}")\n' + ) + for name in ("s1", "s2", "s3") + } + contracts: dict[str, str] = { + f"models/base_{name}.sql": ( + f'MODEL (\n description "Contracted orders {name}",\n contract enforced,\n' + f" columns (customer_id (type INTEGER), amount (type DOUBLE{nullable})),\n);\n\n" + 'SELECT customer_id, amount FROM __source("s1")\n' + ) + for name, nullable in (("p", ""), ("q", ", nullable false")) + } + dependants: dict[str, str] = { + f"models/d_{name}.sql": ( + 'MODEL (description "Contracted orders with null handling");\n\n' + "SELECT customer_id, amount, COALESCE(amount, 0) AS filled, amount + 1 AS bumped\n" + f'FROM __ref("base_{name}")\n' + ) + for name in ("p", "q") + } + return { + "sqlbuild_project.toml": _PROJECT_TOML, + "sources/raw.yml": sources, + **readers, + **contracts, + **dependants, + } + + def started_sessions(*, monkeypatch: pytest.MonkeyPatch) -> list[Any]: """Record every native session started; return the list they are appended to.""" diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py index 34e3dfddb0..88db5db6d2 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py @@ -40,6 +40,7 @@ deferral_kinds, failing_provide_sessions, generated_analysis_files, + nullability_analysis_files, native_pivot_proofs, pivot_project_files, shared_analysis_files, @@ -114,14 +115,25 @@ def test_given_generated_projects_when_analysing_natively_then_matches_python( [ SharedAnalysisTestCase( description="equal regional queries, missing-column readers and a unique summary", - regions=("east", "west", "north", "south"), - inexact_regions=("east", "west", "north"), + files=shared_analysis_files( + regions=("east", "west", "north", "south"), + inexact_regions=("east", "west", "north"), + ), dialects=("duckdb", "snowflake"), expected_analysed=24, expected_shared=22, expected_reanalysed=6, expected_unshared=2, - ) + ), + SharedAnalysisTestCase( + description="equal queries over inputs differing only in nullability", + files=nullability_analysis_files(), + dialects=("duckdb",), + expected_analysed=7, + expected_shared=4, + expected_reanalysed=0, + expected_unshared=3, + ), ], ids=lambda case: case.description, ) @@ -131,10 +143,7 @@ def test_given_equal_model_queries_when_analysing_natively_then_shares_and_match record_dir: Path = tmp_path / "records" monkeypatch.setenv(ANALYSIS_RECORD_DIR_ENV_VAR, str(record_dir)) inputs: CompileProjectInputs = compile_inputs( - project_dir=tmp_path / "project", - files=shared_analysis_files( - regions=test_case.regions, inexact_regions=test_case.inexact_regions - ), + project_dir=tmp_path / "project", files=test_case.files ) sessions: list[Any] = started_sessions(monkeypatch=monkeypatch) parity: AnalysisParity = AnalysisParity() From 8b9c7baaf8ec1085aa89bb28347637d38d9aa188 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:49:48 +0100 Subject: [PATCH 9/9] style(test): wrap the nullability reader SQL and sort imports --- .../src/sqlbuild/compiler/analysis_session/helpers.py | 4 ++-- .../compiler/analysis_session/test_native_model_analysis.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py index 645bb8944f..2438a8bcf3 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/helpers.py @@ -405,8 +405,8 @@ def nullability_analysis_files() -> dict[str, str]: readers: dict[str, str] = { f"models/m_{name}.sql": ( 'MODEL (description "Orders read with null handling");\n\n' - "SELECT customer_id, amount, COALESCE(amount, 0) AS filled, amount IS NULL AS missing,\n" - f' amount + 1 AS bumped\nFROM __source("{name}")\n' + "SELECT customer_id, amount, COALESCE(amount, 0) AS filled,\n" + f' amount IS NULL AS missing, amount + 1 AS bumped\nFROM __source("{name}")\n' ) for name in ("s1", "s2", "s3") } diff --git a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py index 88db5db6d2..2bc06c888f 100644 --- a/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py +++ b/tests/integration/src/sqlbuild/compiler/analysis_session/test_native_model_analysis.py @@ -40,8 +40,8 @@ deferral_kinds, failing_provide_sessions, generated_analysis_files, - nullability_analysis_files, native_pivot_proofs, + nullability_analysis_files, pivot_project_files, shared_analysis_files, started_sessions,