From 20022bb3a29943460bd604fac5f9b5e1979ff2bf Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Wed, 28 Oct 2026 23:07:03 -0400 Subject: [PATCH] perf(linter/import/no-cycle): reuse traversal buffers between imports (#27481) Each time we do another module graph traversal, we throw away any record of files that might've been visited already from a previous traversal. This means we also allocate a new list of visited records each time, resulting in an initially empty array that must grow repeatedly which causes reallocations. With this PR, the space for storing the traversed module record paths is reused between traversals so it only uses as much space as the single largest traversal, instead of being based on the total number of traversals. So, we have much fewer reallocations. Running on `vscode`: Metric | Before | After | Reduction -- | -- | -- | -- Allocation calls | 16,807,784 | 16,744,378 | -0.4% Reallocation calls | 92,751 | 66,763 | -28.0% Allocated bytes | 1,506,926,944 B | 1,360,327,228 B | -9.7% Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/oxc_linter/src/module_graph_visitor.rs | 58 ++++++++++++++++--- .../oxc_linter/src/rules/import/no_cycle.rs | 7 ++- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/module_graph_visitor.rs b/crates/oxc_linter/src/module_graph_visitor.rs index 4f2f3e7d32..c10302e8f4 100644 --- a/crates/oxc_linter/src/module_graph_visitor.rs +++ b/crates/oxc_linter/src/module_graph_visitor.rs @@ -20,6 +20,7 @@ type LeaveFn<'a> = dyn FnMut(ModulePair, &ModuleRecord) + 'a; /// A builder for creating visitors that walk through the module graph pub struct ModuleGraphVisitorBuilder<'a, T> { max_depth: u32, + traversed: FxHashSet, filter: Option>>, event: Option>>, enter: Option>>, @@ -60,6 +61,14 @@ impl<'a, T> ModuleGraphVisitorBuilder<'a, T> { self } + /// Reuses a previous traversal's set, clearing its contents but retaining its capacity. + #[must_use] + pub fn reuse_traversed(mut self, mut traversed: FxHashSet) -> Self { + traversed.clear(); + self.traversed = traversed; + self + } + /// Sets the filter closure. #[must_use] pub fn filter bool) + 'a>(mut self, filter: F) -> Self { @@ -84,11 +93,8 @@ impl<'a, T> ModuleGraphVisitorBuilder<'a, T> { module: &ModuleRecord, visit: V, ) -> ModuleGraphVisitResult { - let mut visitor = ModuleGraphVisitor { - traversed: FxHashSet::default(), - depth: 0, - max_depth: self.max_depth, - }; + let mut visitor = + ModuleGraphVisitor { traversed: self.traversed, depth: 0, max_depth: self.max_depth }; let filter = self.filter.unwrap_or_else(|| Box::new(|_, _| true)); let event = self.event.unwrap_or_else(|| Box::new(|_, _, _| {})); let enter = self.enter.unwrap_or_else(|| Box::new(|_, _| {})); @@ -104,6 +110,7 @@ impl Default for ModuleGraphVisitorBuilder<'_, T> { fn default() -> Self { Self { max_depth: u32::MAX, + traversed: FxHashSet::default(), filter: None, event: None, enter: None, @@ -115,13 +122,13 @@ impl Default for ModuleGraphVisitorBuilder<'_, T> { pub struct ModuleGraphVisitResult { pub result: T, - pub _traversed: FxHashSet, + pub traversed: FxHashSet, pub _max_depth: u32, } impl ModuleGraphVisitResult { fn with_result(result: T, visitor: ModuleGraphVisitor) -> Self { - Self { result, _traversed: visitor.traversed, _max_depth: visitor.max_depth } + Self { result, traversed: visitor.traversed, _max_depth: visitor.max_depth } } } @@ -235,3 +242,40 @@ impl ModuleGraphVisitor { accumulator } } + +#[test] +fn test_reuse_traversed_clears_contents_and_retains_capacity() { + let mut root = ModuleRecord::default(); + root.resolved_absolute_path = PathBuf::from("root.js"); + let root = Arc::new(root); + let mut dependency = ModuleRecord::default(); + dependency.resolved_absolute_path = PathBuf::from("dependency.js"); + let dependency = Arc::new(dependency); + root.write_loaded_modules() + .insert(CompactStr::from("./dependency.js"), Arc::downgrade(&dependency)); + dependency.write_loaded_modules().insert(CompactStr::from("./root.js"), Arc::downgrade(&root)); + + let mut traversed = FxHashSet::default(); + traversed.reserve(16); + traversed.insert(dependency.resolved_absolute_path.clone()); + let capacity = traversed.capacity(); + + for stop_early in [true, false, true, false] { + let builder = ModuleGraphVisitorBuilder::default().reuse_traversed(traversed); + assert!(builder.traversed.is_empty()); + assert_eq!(builder.traversed.capacity(), capacity); + + let result = builder.visit_fold(0, &root, |count, _, _| { + if stop_early { + VisitFoldWhile::Stop(count + 1) + } else { + VisitFoldWhile::Next(count + 1) + } + }); + let expected = if stop_early { 1 } else { 2 }; + assert_eq!(result.result, expected); + assert_eq!(result.traversed.len(), expected); + assert_eq!(result.traversed.capacity(), capacity); + traversed = result.traversed; + } +} diff --git a/crates/oxc_linter/src/rules/import/no_cycle.rs b/crates/oxc_linter/src/rules/import/no_cycle.rs index 4614ac3b5b..319160b599 100644 --- a/crates/oxc_linter/src/rules/import/no_cycle.rs +++ b/crates/oxc_linter/src/rules/import/no_cycle.rs @@ -167,6 +167,8 @@ impl Rule for NoCycle { .collect::>(); direct_imports.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + let mut stack = Vec::new(); + let mut traversed = FxHashSet::default(); for (key, loaded_module_record) in direct_imports { if !self.should_traverse_module(&key, &loaded_module_record, module_record) { continue; @@ -174,7 +176,8 @@ impl Rule for NoCycle { let requested_module = module_record.requested_modules[&key][0]; let span = requested_module.span; - let mut stack = vec![(key.clone(), Arc::clone(&loaded_module_record))]; + stack.clear(); + stack.push((key.clone(), Arc::clone(&loaded_module_record))); if loaded_module_record.resolved_absolute_path == *needle { ctx.diagnostic(self_referencing_cycle_diagnostic(span, requested_module.is_import)); @@ -183,6 +186,7 @@ impl Rule for NoCycle { let visitor_result = ModuleGraphVisitorBuilder::default() .max_depth(self.max_depth.saturating_sub(1)) + .reuse_traversed(traversed) .filter(|(key, val), parent| self.should_traverse_module(key, val, parent)) .event(|event, (key, val), _| match event { ModuleGraphVisitorEvent::Enter => { @@ -200,6 +204,7 @@ impl Rule for NoCycle { } }); + traversed = visitor_result.traversed; if visitor_result.result { ctx.diagnostic(no_cycle_diagnostic( span,