From 485b43dbb86bac456585163a3822cdf8d5503b85 Mon Sep 17 00:00:00 2001 From: Jonathan Reyes Date: Sun, 11 Jan 2026 16:20:06 +0000 Subject: [PATCH] perf: optimize get_element from O(n) to O(1) lookup The elements_by_id index was storing (path -> vec_index) but the get_element method was discarding the index and doing a linear search through the element vectors anyway. This change: - Stores (ElementType, index) in elements_by_id instead of just index - Rewrites get_element to use direct array indexing based on the stored element type, achieving true O(1) lookup - Uses enumerate() instead of manual index tracking in build_indexes For models with many elements, this provides significant performance improvement for element lookups. Also includes the conditional asset test fix from PR #8. Co-Authored-By: Claude Opus 4.5 --- src/exporter/html.rs | 9 ++++++- src/model/mod.rs | 62 ++++++++++++++++---------------------------- 2 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/exporter/html.rs b/src/exporter/html.rs index fb5b01e..4cb0c55 100644 --- a/src/exporter/html.rs +++ b/src/exporter/html.rs @@ -186,6 +186,14 @@ mod tests { #[test] fn test_export_html_asset_files() { + // Check if frontend assets are available (only present after `make build`) + let has_embedded_assets = StaticAssets::iter().any(|f| f.starts_with("assets/")); + if !has_embedded_assets { + // Skip test when frontend hasn't been built - this is expected during + // development with `cargo test`. Use `make test` to run with full assets. + return; + } + let model = create_test_model(); let temp_dir = TempDir::new().unwrap(); let output_dir = temp_dir.path().to_str().unwrap(); @@ -195,7 +203,6 @@ mod tests { let assets_dir = temp_dir.path().join("assets"); let entries: Vec<_> = fs::read_dir(&assets_dir).unwrap().collect(); - // Check that CSS and JS files exist (Vite uses hashed filenames) let has_css = entries.iter().any(|e| { e.as_ref() .unwrap() diff --git a/src/model/mod.rs b/src/model/mod.rs index 6da352e..420ee79 100644 --- a/src/model/mod.rs +++ b/src/model/mod.rs @@ -36,8 +36,9 @@ pub struct Model { pub options: Options, // Indexes for fast lookup (not serialized) + // Stores (ElementType, index) for O(1) lookup by path #[serde(skip)] - elements_by_id: HashMap, + elements_by_id: HashMap, #[serde(skip)] elements_by_type: HashMap>, #[serde(skip)] @@ -84,13 +85,13 @@ impl Model { self.incoming_rels.clear(); // Index persons - for idx in 0..self.persons.len() { - let p = &self.persons[idx]; + for (idx, p) in self.persons.iter().enumerate() { let path = p.get_full_path(); if self.elements_by_id.contains_key(&path) { return Err(ModelError::DuplicateElement(path)); } - self.elements_by_id.insert(path.clone(), idx); + self.elements_by_id + .insert(path.clone(), (ElementType::Person, idx)); self.elements_by_type .entry(ElementType::Person) .or_default() @@ -105,13 +106,13 @@ impl Model { } // Index systems - for idx in 0..self.systems.len() { - let s = &self.systems[idx]; + for (idx, s) in self.systems.iter().enumerate() { let path = s.get_full_path(); if self.elements_by_id.contains_key(&path) { return Err(ModelError::DuplicateElement(path)); } - self.elements_by_id.insert(path.clone(), idx); + self.elements_by_id + .insert(path.clone(), (ElementType::System, idx)); self.elements_by_type .entry(ElementType::System) .or_default() @@ -126,13 +127,13 @@ impl Model { } // Index containers - for idx in 0..self.containers.len() { - let c = &self.containers[idx]; + for (idx, c) in self.containers.iter().enumerate() { let path = c.get_full_path(); if self.elements_by_id.contains_key(&path) { return Err(ModelError::DuplicateElement(path)); } - self.elements_by_id.insert(path.clone(), idx); + self.elements_by_id + .insert(path.clone(), (ElementType::Container, idx)); self.elements_by_type .entry(ElementType::Container) .or_default() @@ -152,13 +153,13 @@ impl Model { } // Index components - for idx in 0..self.components.len() { - let c = &self.components[idx]; + for (idx, c) in self.components.iter().enumerate() { let path = c.get_full_path(); if self.elements_by_id.contains_key(&path) { return Err(ModelError::DuplicateElement(path)); } - self.elements_by_id.insert(path.clone(), idx); + self.elements_by_id + .insert(path.clone(), (ElementType::Component, idx)); self.elements_by_type .entry(ElementType::Component) .or_default() @@ -193,34 +194,15 @@ impl Model { Ok(()) } - /// Returns an element by its full path + /// Returns an element by its full path using O(1) indexed lookup pub fn get_element(&self, path: &str) -> Option<&dyn Element> { - let _idx = self.elements_by_id.get(path)?; - - // Determine type from path structure - let parts: Vec<&str> = path.split('.').collect(); - match parts.len() { - 1 => { - // Could be Person or System - check systems first - if let Some(sys) = self.systems.iter().find(|s| s.get_full_path() == path) { - return Some(sys as &dyn Element); - } - self.persons - .iter() - .find(|p| p.get_full_path() == path) - .map(|p| p as &dyn Element) - } - 2 => self - .containers - .iter() - .find(|c| c.get_full_path() == path) - .map(|c| c as &dyn Element), - 3 => self - .components - .iter() - .find(|c| c.get_full_path() == path) - .map(|c| c as &dyn Element), - _ => None, + let (element_type, idx) = self.elements_by_id.get(path)?; + + match element_type { + ElementType::Person => self.persons.get(*idx).map(|p| p as &dyn Element), + ElementType::System => self.systems.get(*idx).map(|s| s as &dyn Element), + ElementType::Container => self.containers.get(*idx).map(|c| c as &dyn Element), + ElementType::Component => self.components.get(*idx).map(|c| c as &dyn Element), } }