diff --git a/rust/src/graph/graph_manipulation.rs b/rust/src/graph/graph_manipulation.rs index 41428236..caed21c1 100644 --- a/rust/src/graph/graph_manipulation.rs +++ b/rust/src/graph/graph_manipulation.rs @@ -1,4 +1,6 @@ -use crate::graph::{Graph, ImportDetails, Module, ModuleIterator, ModuleToken, MODULE_NAMES}; +use crate::graph::{ + Graph, ImportDetails, Module, ModuleIterator, ModuleToken, IMPORT_LINE_CONTENTS, MODULE_NAMES, +}; use rustc_hash::FxHashSet; use slotmap::secondary::Entry; @@ -31,7 +33,7 @@ impl Graph { } else { let module = self.modules.insert_with_key(|token| Module { token, - name, + interned_name: name, is_invisible: !ancestor_names.is_empty(), is_squashed: false, }); @@ -116,7 +118,7 @@ impl Graph { &mut self, importer: ModuleToken, imported: ModuleToken, - line_number: usize, + line_number: u32, line_contents: &str, ) { self.imports @@ -129,10 +131,14 @@ impl Graph { .unwrap() .or_default() .insert(importer); - self.import_details - .entry((importer, imported)) - .or_default() - .insert(ImportDetails::new(line_number, line_contents.to_owned())); + { + let mut interner = IMPORT_LINE_CONTENTS.write().unwrap(); + let line_contents = interner.get_or_intern(line_contents); + self.import_details + .entry((importer, imported)) + .or_default() + .insert(ImportDetails::new(line_number, line_contents)); + } } pub fn remove_import(&mut self, importer: ModuleToken, imported: ModuleToken) { diff --git a/rust/src/graph/mod.rs b/rust/src/graph/mod.rs index cdbd5380..8a97b2bd 100644 --- a/rust/src/graph/mod.rs +++ b/rust/src/graph/mod.rs @@ -19,6 +19,8 @@ pub(crate) mod pathfinding; lazy_static! { static ref MODULE_NAMES: RwLock> = RwLock::new(StringInterner::default()); + static ref IMPORT_LINE_CONTENTS: RwLock> = + RwLock::new(StringInterner::default()); static ref EMPTY_MODULE_TOKENS: FxHashSet = FxHashSet::default(); static ref EMPTY_IMPORT_DETAILS: FxHashSet = FxHashSet::default(); static ref EMPTY_IMPORTS: FxHashSet<(ModuleToken, ModuleToken)> = FxHashSet::default(); @@ -32,7 +34,7 @@ pub struct Module { token: ModuleToken, #[getset(get_copy = "pub")] - name: DefaultSymbol, + interned_name: DefaultSymbol, // Invisible modules exist in the hierarchy but haven't been explicitly added to the graph. #[getset(get_copy = "pub")] @@ -43,9 +45,9 @@ pub struct Module { } impl Module { - pub fn name_as_string(&self) -> String { + pub fn name(&self) -> String { let interner = MODULE_NAMES.read().unwrap(); - interner.resolve(self.name).unwrap().to_owned() + interner.resolve(self.interned_name).unwrap().to_owned() } } @@ -102,13 +104,13 @@ pub trait ModuleIterator<'a>: Iterator + Sized { self.map(|m| m.token) } - fn names(self) -> impl Iterator { - self.map(|m| m.name) + fn interned_names(self) -> impl Iterator { + self.map(|m| m.interned_name) } - fn names_as_strings(self) -> impl Iterator { + fn names(self) -> impl Iterator { let interner = MODULE_NAMES.read().unwrap(); - self.map(move |m| interner.resolve(m.name).unwrap().to_owned()) + self.map(move |m| interner.resolve(m.interned_name).unwrap().to_owned()) } fn visible(self) -> impl ModuleIterator<'a> { @@ -129,9 +131,18 @@ impl<'a, T: Iterator> ModuleTokenIterator<'a> for T {} #[derive(Debug, Clone, PartialEq, Eq, Hash, new, Getters, CopyGetters)] pub struct ImportDetails { #[getset(get_copy = "pub")] - line_number: usize, + line_number: u32, - #[new(into)] - #[getset(get = "pub")] - line_contents: String, + #[getset(get_copy = "pub")] + interned_line_contents: DefaultSymbol, +} + +impl ImportDetails { + pub fn line_contents(&self) -> String { + let interner = IMPORT_LINE_CONTENTS.read().unwrap(); + interner + .resolve(self.interned_line_contents) + .unwrap() + .to_owned() + } } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d293f852..ff8a0588 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -49,11 +49,7 @@ impl GraphWrapper { } pub fn get_modules(&self) -> HashSet { - self._graph - .all_modules() - .visible() - .names_as_strings() - .collect() + self._graph.all_modules().visible().names().collect() } pub fn contains_module(&self, name: &str) -> bool { @@ -115,7 +111,7 @@ impl GraphWrapper { &mut self, importer: &str, imported: &str, - line_number: Option, + line_number: Option, line_contents: Option<&str>, ) { let importer = self._graph.get_or_add_module(importer).token(); @@ -156,7 +152,7 @@ impl GraphWrapper { ._graph .get_module_children(module.token()) .visible() - .names_as_strings() + .names() .collect()) } @@ -169,7 +165,7 @@ impl GraphWrapper { ._graph .get_module_descendants(module.token()) .visible() - .names_as_strings() + .names() .collect()) } @@ -195,7 +191,7 @@ impl GraphWrapper { .iter() .into_module_iterator(&self._graph) .visible() - .names_as_strings() + .names() .collect()) } @@ -207,7 +203,7 @@ impl GraphWrapper { .iter() .into_module_iterator(&self._graph) .visible() - .names_as_strings() + .names() .collect()) } @@ -234,10 +230,10 @@ impl GraphWrapper { .iter() .map(|import_details| { ImportDetails::new( - importer.name_as_string(), - imported.name_as_string(), + importer.name(), + imported.name(), import_details.line_number(), - import_details.line_contents().to_owned(), + import_details.line_contents(), ) }) .sorted() @@ -274,7 +270,7 @@ impl GraphWrapper { .iter() .into_module_iterator(&self._graph) .visible() - .names_as_strings() + .names() .collect()) } @@ -292,7 +288,7 @@ impl GraphWrapper { .iter() .into_module_iterator(&self._graph) .visible() - .names_as_strings() + .names() .collect()) } @@ -311,7 +307,7 @@ impl GraphWrapper { chain .iter() .into_module_iterator(&self._graph) - .names_as_strings() + .names() .collect() })) } @@ -348,7 +344,7 @@ impl GraphWrapper { chain .iter() .into_module_iterator(&self._graph) - .names_as_strings() + .names() .collect::>(), ) .unwrap() @@ -391,14 +387,8 @@ impl GraphWrapper { .into_iter() .map(|dep| { PackageDependency::new( - self._graph - .get_module(*dep.importer()) - .unwrap() - .name_as_string(), - self._graph - .get_module(*dep.imported()) - .unwrap() - .name_as_string(), + self._graph.get_module(*dep.importer()).unwrap().name(), + self._graph.get_module(*dep.imported()).unwrap().name(), dep.routes() .iter() .map(|route| { @@ -406,17 +396,17 @@ impl GraphWrapper { route .heads() .iter() - .map(|m| self._graph.get_module(*m).unwrap().name_as_string()) + .map(|m| self._graph.get_module(*m).unwrap().name()) .collect(), route .middle() .iter() - .map(|m| self._graph.get_module(*m).unwrap().name_as_string()) + .map(|m| self._graph.get_module(*m).unwrap().name()) .collect(), route .tails() .iter() - .map(|m| self._graph.get_module(*m).unwrap().name_as_string()) + .map(|m| self._graph.get_module(*m).unwrap().name()) .collect(), ) }) @@ -460,10 +450,7 @@ impl GraphWrapper { ) -> Vec> { let containers = match containers.is_empty() { true => vec![None], - false => containers - .iter() - .map(|c| Some(c.name_as_string())) - .collect(), + false => containers.iter().map(|c| Some(c.name())).collect(), }; let mut levels_by_container: Vec> = vec![]; @@ -553,7 +540,7 @@ impl GraphWrapper { struct ImportDetails { importer: String, imported: String, - line_number: usize, + line_number: u32, line_contents: String, } diff --git a/tests/benchmarking/test_benchmarking.py b/tests/benchmarking/test_benchmarking.py index 4cedd334..b855f7d6 100644 --- a/tests/benchmarking/test_benchmarking.py +++ b/tests/benchmarking/test_benchmarking.py @@ -20,7 +20,12 @@ def large_graph(): for importer, importeds in graph_dict.items(): graph.add_module(importer) for imported in importeds: - graph.add_import(importer=importer, imported=imported) + graph.add_import( + importer=importer, + imported=imported, + line_number=1, + line_contents=f"import {imported}", + ) return graph