From 67cfd901fb6beb32b93a5ca59d6ca2848ea98977 Mon Sep 17 00:00:00 2001 From: Felipe705x Date: Thu, 23 Jul 2026 23:04:51 -0400 Subject: [PATCH] =?UTF-8?q?perf(typing):=20M07=20=E2=80=94=20owning=20env?= =?UTF-8?q?=20meet=20kills=20quadratic=20clones;=20union=20ptr=20fast=20pa?= =?UTF-8?q?th?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New lattice-op counters (vt_meets, vt_joins, env_bindings_copied) localized the remaining cost precisely: - chains do ZERO VariableType meets — their cost was env cloning: every Concat cloned the accumulated environment, O(vars) String-key clones per hop, quadratic along a chain (272 copies for chain_16); - subq/optional shapes spend their time in real meet/join walks over rich refined types (22–44 per check) — targeted next, not here. Changes: - TypeEnvironment::meet_owned consumes the left env (Concat/Join/ match-chain/subquery folds own their accumulator). Merged bindings stage in a scratch Vec applied on success; the Err path hands the environment back untouched, preserving the exact keep-previous-env-on-error behavior. meet(&,&) remains as the cloning wrapper. - warn_for_collapsed_bindings walked all merged keys; only SHARED keys can collapse (one-sided keys pass through meet untouched, so their "empty" was pre-existing and the old walk skipped them — its one-sided message arms were unreachable). Now takes the stashed shared bindings, warnings byte-identical. - TypeEnvironment::union: Rc::ptr_eq fast path when both arms hold the same shared binding (join(v,v) collapses to v) — common since both arms get the same refine-cache Rc. Idle-machine medians vs M06: - chain_16 64.2 → 46.4 us (−28%) chain_8 28.3 → 22.1 us (−22%) - chain_dir_16 73.4 → 55.1 us (−25%) anydir_8 54.1 → 40.5 us (−25%) - union_8 59.0 → 50.0 us (−15%) - subq_exists / multi_optional unchanged (their cost is the lattice meets; that is the next milestone) - chk/parse: chains now 2.7–4.1× Full sweep 80 targets green. Co-Authored-By: Claude Fable 5 --- src/bin/pattern_typecheck.rs | 6 +- src/typing/checker.rs | 134 ++++++++++++++++++++------------- src/typing/stats.rs | 24 ++++++ src/typing/type_environment.rs | 55 ++++++++++---- src/typing/variable_type.rs | 2 + 5 files changed, 154 insertions(+), 67 deletions(-) diff --git a/src/bin/pattern_typecheck.rs b/src/bin/pattern_typecheck.rs index 5f4585f..0fc79bd 100644 --- a/src/bin/pattern_typecheck.rs +++ b/src/bin/pattern_typecheck.rs @@ -480,6 +480,7 @@ fn main() { "case,category,expected,got,check_med_ns,check_min_ns,parse_med_ns,\ refine_calls,refine_cache_hits,node_scanned,edge_scanned,refine_to_nodes,\ env_meets,env_unions,env_outer_joins,env_to_groups,pt_meets,\ + vt_meets,vt_joins,env_copied,\ phase_pattern_ns,phase_rep_ns,phase_group_by_ns,phase_returns_ns,phase_order_by_ns,\ status" ) @@ -487,7 +488,7 @@ fn main() { for r in &rows { writeln!( f, - "{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{}", + "{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{},{}", r.id, r.category, r.expected, @@ -505,6 +506,9 @@ fn main() { r.st.env_outer_join_calls, r.st.env_to_group_calls, r.st.pathtype_meet_calls, + r.st.vt_meet_calls, + r.st.vt_join_calls, + r.st.env_bindings_copied, r.phases[0], r.phases[1], r.phases[2], diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 48bc014..0544700 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -444,15 +444,16 @@ impl Typechecker { let r = self.check_path_pattern(m.pattern()); env = match m { MatchStatement::Simple { .. } => { - match TypeEnvironment::meet(&self.schema, &env, &r.env) { + let left_shared = shared_bindings(&env, &r.env); + match TypeEnvironment::meet_owned(&self.schema, env, &r.env) { Ok(e) => { - self.warn_for_collapsed_bindings(&e, &env, &r.env); + self.warn_for_collapsed_bindings_shared(&e, &left_shared, &r.env); e } - Err(e) => { + Err((prev, e)) => { self.errors .push(format!("Concatenation of contexts failed: {}", e)); - env + prev } } } @@ -594,16 +595,22 @@ impl Typechecker { let r1 = self.check_path_pattern(p1); let r2 = self.check_path_pattern(p2); - let cm = match TypeEnvironment::meet(&self.schema, &r1.env, &r2.env) { + // The warning pass only needs the SHARED keys' originals; + // meet_owned consumes r1's env without cloning it, so + // stash the left-side bindings for vars the right also + // binds (few, usually zero or one). + let left_shared = shared_bindings(&r1.env, &r2.env); + + let cm = match TypeEnvironment::meet_owned(&self.schema, r1.env, &r2.env) { Ok(env) => env, - Err(e) => { + Err((env, e)) => { self.errors .push(format!("Concatenation of contexts failed: {}", e)); - r1.env.clone() + env } }; - self.warn_for_collapsed_bindings(&cm, &r1.env, &r2.env); + self.warn_for_collapsed_bindings_shared(&cm, &left_shared, &r2.env); let p = PathSummary::meet(&self.schema, &r1.path, &r2.path); TypecheckResult::new(p, cm) @@ -618,16 +625,17 @@ impl Typechecker { let r1 = self.check_path_pattern(p1); let r2 = self.check_path_pattern(p2); - let cm = match TypeEnvironment::meet(&self.schema, &r1.env, &r2.env) { + let left_shared = shared_bindings(&r1.env, &r2.env); + let cm = match TypeEnvironment::meet_owned(&self.schema, r1.env, &r2.env) { Ok(env) => env, - Err(e) => { + Err((env, e)) => { self.errors .push(format!("Concatenation of contexts failed: {}", e)); - r1.env.clone() + env } }; - self.warn_for_collapsed_bindings(&cm, &r1.env, &r2.env); + self.warn_for_collapsed_bindings_shared(&cm, &left_shared, &r2.env); let p = if r1.path.is_unsatisfiable() || r2.path.is_unsatisfiable() { PathSummary::zero() @@ -1091,20 +1099,32 @@ impl Typechecker { if path.is_none() { path = Some(r.path.clone()); } - let base = env.as_ref().unwrap_or(outer); let next = match m { - MatchStatement::Simple { .. } => { - match TypeEnvironment::meet(&self.schema, base, &r.env) { + MatchStatement::Simple { .. } => match env.take() { + // Owned accumulator: meet without cloning; the error + // path hands the previous environment back. + Some(base) => match TypeEnvironment::meet_owned(&self.schema, base, &r.env) { + Ok(e) => Some(e), + Err((prev, msg)) => { + self.errors.push(format!( + "EXISTS body: concatenation of contexts failed: {msg}" + )); + Some(prev) + } + }, + // First fold step: the base is the borrowed outer env. + None => match TypeEnvironment::meet(&self.schema, outer, &r.env) { Ok(e) => Some(e), Err(msg) => { self.errors.push(format!( "EXISTS body: concatenation of contexts failed: {msg}" )); - None // keep the previous environment + None // keep the previous (outer) environment } - } - } + }, + }, MatchStatement::Optional { .. } => { + let base = env.as_ref().unwrap_or(outer); Some(TypeEnvironment::outer_join(&self.schema, base, &r.env)) } }; @@ -1193,47 +1213,38 @@ impl Typechecker { } } - /// After meeting two pattern contexts, surface any variable whose - /// type collapsed to bottom. Each side's contribution is shown so - /// the user can see the conflict directly. Pre-existing empties - /// (already empty in `left` or `right`) are skipped to avoid - /// double-warning. - fn warn_for_collapsed_bindings( + /// After meeting two pattern contexts, surface any SHARED variable + /// whose type collapsed to bottom. Only shared keys can collapse: a + /// one-sided key passes through `meet` untouched, so if its merged + /// binding is empty it was already empty on its own side — which the + /// previous all-keys walk skipped as a "pre-existing empty" (its + /// one-sided message arms were unreachable for exactly this reason). + /// Each side's contribution is shown so the user can see the + /// conflict directly. + fn warn_for_collapsed_bindings_shared( &mut self, merged: &TypeEnvironment, - left: &TypeEnvironment, + left_shared: &[(String, std::rc::Rc)], right: &TypeEnvironment, ) { - for (var, merged_t) in merged.iter() { - if !merged_t.is_empty() { - continue; - } - let l_t = left.get(var); - let r_t = right.get(var); - let l_was_empty = l_t.is_some_and(VariableType::is_empty); - let r_was_empty = r_t.is_some_and(VariableType::is_empty); - if l_was_empty || r_was_empty { + for (var, l) in left_shared { + let merged_t = match merged.get(var) { + Some(t) => t, + None => continue, + }; + if !merged_t.is_empty() || l.is_empty() { continue; } - match (l_t, r_t) { - (Some(l), Some(r)) => self.warnings.push(format!( - "variable {} cannot be both {} and {} under the active schema", - var, - short_var_type(l), - short_var_type(r) - )), - (Some(l), None) => self.warnings.push(format!( - "variable {} bound to {} collapses to empty under the active schema", - var, - short_var_type(l) - )), - (None, Some(r)) => self.warnings.push(format!( - "variable {} bound to {} collapses to empty under the active schema", - var, - short_var_type(r) - )), - (None, None) => {} - } + let r = match right.get(var) { + Some(r) if !r.is_empty() => r, + _ => continue, + }; + self.warnings.push(format!( + "variable {} cannot be both {} and {} under the active schema", + var, + short_var_type(l), + short_var_type(r) + )); } } @@ -1254,6 +1265,23 @@ fn descriptor_type_of(desc: &Option) -> DescriptorType { /// Build the binding environment for a pattern position. Anonymous patterns /// contribute no bindings. +/// The left side's bindings for every key the right side also binds — +/// the inputs `warn_for_collapsed_bindings_shared` needs after +/// `meet_owned` has consumed the left environment. Usually empty or a +/// single entry, so the `Vec` rarely allocates. +fn shared_bindings( + left: &TypeEnvironment, + right: &TypeEnvironment, +) -> Vec<(String, std::rc::Rc)> { + right + .iter() + .filter_map(|(k, _)| { + left.get_shared(k) + .map(|v| (k.clone(), std::rc::Rc::clone(v))) + }) + .collect() +} + fn create_context(desc: &Option, t: std::rc::Rc) -> TypeEnvironment { match desc { Some(d) => TypeEnvironment::create_context_shared(d, t), diff --git a/src/typing/stats.rs b/src/typing/stats.rs index c081581..a8c81af 100644 --- a/src/typing/stats.rs +++ b/src/typing/stats.rs @@ -34,6 +34,12 @@ pub struct TcStats { pub env_to_group_calls: u64, /// Calls to `PathType::meet` (every Concat, recursing through Unions). pub pathtype_meet_calls: u64, + /// Calls to `VariableType::meet` (incl. recursion into Union arms). + pub vt_meet_calls: u64, + /// Calls to `VariableType::join` (incl. list folds). + pub vt_join_calls: u64, + /// Bindings copied by env clones/merges (`String` key + `Rc` bump each). + pub env_bindings_copied: u64, } const ZERO: TcStats = TcStats { @@ -47,6 +53,9 @@ const ZERO: TcStats = TcStats { env_outer_join_calls: 0, env_to_group_calls: 0, pathtype_meet_calls: 0, + vt_meet_calls: 0, + vt_join_calls: 0, + env_bindings_copied: 0, }; thread_local! { @@ -122,3 +131,18 @@ pub(crate) fn record_env_to_group() { pub(crate) fn record_pathtype_meet() { bump(|s| s.pathtype_meet_calls += 1); } + +#[inline] +pub(crate) fn record_vt_meet() { + bump(|s| s.vt_meet_calls += 1); +} + +#[inline] +pub(crate) fn record_vt_join() { + bump(|s| s.vt_join_calls += 1); +} + +#[inline] +pub(crate) fn record_env_bindings_copied(n: usize) { + bump(|s| s.env_bindings_copied += n as u64); +} diff --git a/src/typing/type_environment.rs b/src/typing/type_environment.rs index 4b3055e..a02a3c7 100644 --- a/src/typing/type_environment.rs +++ b/src/typing/type_environment.rs @@ -46,6 +46,12 @@ impl TypeEnvironment { self.bindings.get(key).map(Rc::as_ref) } + /// Like `get`, but exposes the shared binding for callers that need + /// to retain it without deep-cloning. + pub fn get_shared(&self, key: &str) -> Option<&Rc> { + self.bindings.get(key) + } + pub fn keys(&self) -> impl Iterator { self.bindings.keys() } @@ -74,12 +80,16 @@ impl TypeEnvironment { let mut result = HashMap::with_capacity(keys.len()); for key in keys { let merged = match (a.bindings.get(key), b.bindings.get(key)) { - (Some(ta), Some(tb)) => VariableType::join((**ta).clone(), (**tb).clone()), - (Some(ta), None) => VariableType::join((**ta).clone(), VariableType::Null), - (None, Some(tb)) => VariableType::join(VariableType::Null, (**tb).clone()), + // Same shared binding on both sides (common: both arms + // hold the same refine-cache Rc): `join(v, v)` collapses + // to `v`, so share it without cloning or walking. + (Some(ta), Some(tb)) if Rc::ptr_eq(ta, tb) => Rc::clone(ta), + (Some(ta), Some(tb)) => Rc::new(VariableType::join((**ta).clone(), (**tb).clone())), + (Some(ta), None) => Rc::new(VariableType::join((**ta).clone(), VariableType::Null)), + (None, Some(tb)) => Rc::new(VariableType::join(VariableType::Null, (**tb).clone())), (None, None) => unreachable!(), }; - result.insert(key.clone(), Rc::new(merged)); + result.insert(key.clone(), merged); } TypeEnvironment { bindings: result } } @@ -95,26 +105,45 @@ impl TypeEnvironment { a: &TypeEnvironment, b: &TypeEnvironment, ) -> Result { + super::stats::record_env_bindings_copied(a.bindings.len()); + TypeEnvironment::meet_owned(schema, a.clone(), b).map_err(|(_, msg)| msg) + } + + /// `meet` that consumes the left environment instead of cloning it — + /// the checker's Concat/Join/match-chain folds own their accumulator + /// and were paying O(vars) String-key clones per operator (quadratic + /// along a chain). Merged bindings are staged in a scratch `Vec` and + /// applied only on success, so the `Err` case hands `a` back + /// untouched (callers keep the pre-meet environment on error, exactly + /// as the cloning version behaved). + pub fn meet_owned( + schema: &Schema, + mut a: TypeEnvironment, + b: &TypeEnvironment, + ) -> Result { super::stats::record_env_meet(); - let mut result = a.bindings.clone(); + let mut merged: Vec<(&String, Rc)> = Vec::new(); for (key, other) in &b.bindings { - let merged: Rc = match result.get(key) { + match a.bindings.get(key) { Some(self_t) => { let met = VariableType::meet(self_t, other); if met == VariableType::Zero && !self_t.is_empty() && !other.is_empty() { - return Err(format!( + let msg = format!( "Cannot reconcile types for variable {}: {} and {}", key, self_t, other - )); + ); + return Err((a, msg)); } - VariableType::refine_rc(schema, &met) + merged.push((key, VariableType::refine_rc(schema, &met))); } // Right-only key: keep the binding as-is. - None => Rc::clone(other), - }; - result.insert(key.clone(), merged); + None => merged.push((key, Rc::clone(other))), + } + } + for (k, v) in merged { + a.bindings.insert(k.clone(), v); } - Ok(TypeEnvironment { bindings: result }) + Ok(a) } /// Left outer join — the typing operator `Γ₁ ⟕ Γ₂` for OPTIONAL MATCH, diff --git a/src/typing/variable_type.rs b/src/typing/variable_type.rs index 9e72314..4356e54 100644 --- a/src/typing/variable_type.rs +++ b/src/typing/variable_type.rs @@ -114,6 +114,7 @@ impl VariableType { } pub fn meet(a: &VariableType, b: &VariableType) -> VariableType { + super::stats::record_vt_meet(); match (a, b) { (VariableType::Group(ta), VariableType::Group(tb)) => { VariableType::Group(Box::new(VariableType::meet(ta, tb))) @@ -193,6 +194,7 @@ impl VariableType { // --- Join --- pub fn join(a: VariableType, b: VariableType) -> VariableType { + super::stats::record_vt_join(); if a == VariableType::Zero { return b; }