From 9dd4328a93d467a65ee78e5ede0cd957ad2ce81f Mon Sep 17 00:00:00 2001 From: Oliver Wooding Date: Sat, 14 Mar 2026 13:47:34 +0100 Subject: [PATCH] fix: wrap FieldPath inner Vec in Rc for O(1) cloning FieldPath values are frequently cloned when stored in Blame objects during record lowering. By wrapping the inner Vec in Rc, clones become O(1) reference count bumps instead of O(n) vector copies. The push method now pre-allocates with exact capacity instead of clone + push, and the inner field is made private with a segments() accessor to maintain encapsulation. Closes #32 Co-Authored-By: Claude Opus 4.6 --- crates/gnomon-db/src/eval.rs | 2 +- crates/gnomon-db/src/eval/interned.rs | 19 +++++++++++++++---- crates/gnomon-db/src/eval/render.rs | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/gnomon-db/src/eval.rs b/crates/gnomon-db/src/eval.rs index 6901c8d..66505cd 100644 --- a/crates/gnomon-db/src/eval.rs +++ b/crates/gnomon-db/src/eval.rs @@ -454,7 +454,7 @@ mod tests { let r = expect_single_decl(&result); let uid_name = FieldName::new(&db, "uid".to_string()); let blamed_value = r.get(&db, &uid_name).unwrap(); - assert_eq!(blamed_value.blame.path.0.len(), 1); + assert_eq!(blamed_value.blame.path.segments().len(), 1); } // ── Every expression lowering ──────────────────────────────── diff --git a/crates/gnomon-db/src/eval/interned.rs b/crates/gnomon-db/src/eval/interned.rs index 6183864..7708396 100644 --- a/crates/gnomon-db/src/eval/interned.rs +++ b/crates/gnomon-db/src/eval/interned.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::rc::Rc; use crate::input::SourceFile; @@ -23,20 +24,25 @@ pub enum PathSegment<'db> { /// A path into a record structure, e.g. `[Field("alerts"), Index(0), Field("trigger")]`. /// +/// Uses `Rc` for O(1) cloning. Paths are frequently cloned when stored in +/// `Blame` values, so cheap clones avoid the O(n) copy that a bare `Vec` +/// would require on every clone. +/// /// Not salsa-interned because it contains `FieldName<'db>` values that carry a /// non-`'static` lifetime. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct FieldPath<'db>(pub Vec>); +pub struct FieldPath<'db>(Rc>>); impl<'db> FieldPath<'db> { pub fn root() -> Self { - Self(Vec::new()) + Self(Rc::new(Vec::new())) } pub fn push(&self, segment: PathSegment<'db>) -> Self { - let mut segments = self.0.clone(); + let mut segments = Vec::with_capacity(self.0.len() + 1); + segments.extend_from_slice(&self.0); segments.push(segment); - Self(segments) + Self(Rc::new(segments)) } pub fn field(&self, name: FieldName<'db>) -> Self { @@ -46,6 +52,11 @@ impl<'db> FieldPath<'db> { pub fn index(&self, i: usize) -> Self { self.push(PathSegment::Index(i)) } + + /// Returns a reference to the path segments. + pub fn segments(&self) -> &[PathSegment<'db>] { + &self.0 + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/crates/gnomon-db/src/eval/render.rs b/crates/gnomon-db/src/eval/render.rs index 3787190..f548e10 100644 --- a/crates/gnomon-db/src/eval/render.rs +++ b/crates/gnomon-db/src/eval/render.rs @@ -122,7 +122,7 @@ impl<'db> RenderWithDb<'db> for PathSegment<'db> { impl<'db> RenderWithDb<'db> for FieldPath<'db> { fn render_fmt(&self, f: &mut fmt::Formatter<'_>, db: &'db dyn Db) -> fmt::Result { - for (i, segment) in self.0.iter().enumerate() { + for (i, segment) in self.segments().iter().enumerate() { if i > 0 && matches!(segment, PathSegment::Field(_)) { write!(f, ".")?; }