Repository: vercel/next.js
Pull request: perf(turbopack): Remove extra indirection

### What?

Remove `VisitorFactory` in favor of directly using `AstModifier`.

### Why?

We don't need to allocate twice in this case.

Closes PACK-4671

Changed files:
filename: turbopack/crates/turbopack-ecmascript/src/code_gen.rs
patch: @@ -39,7 +39,7 @@ use crate::references::{
 pub struct CodeGeneration {
     /// ast nodes matching the span will be visitor by the visitor
     pub root_visitors: Vec<Box<dyn PassFactory>>,
-    pub visitors: Vec<(Vec<AstParentKind>, Box<dyn VisitorFactory>)>,
+    pub visitors: Vec<(Vec<AstParentKind>, Box<dyn AstModifier>)>,
     pub hoisted_stmts: Vec<CodeGenerationHoistedStmt>,
     pub early_hoisted_stmts: Vec<CodeGenerationHoistedStmt>,
 }
@@ -53,7 +53,7 @@ impl CodeGeneration {
 
     pub fn new(
         root_visitors: Vec<Box<dyn PassFactory>>,
-        visitors: Vec<(Vec<AstParentKind>, Box<dyn VisitorFactory>)>,
+        visitors: Vec<(Vec<AstParentKind>, Box<dyn AstModifier>)>,
         hoisted_stmts: Vec<CodeGenerationHoistedStmt>,
         early_hoisted_stmts: Vec<CodeGenerationHoistedStmt>,
     ) -> Self {
@@ -72,7 +72,7 @@ impl CodeGeneration {
         }
     }
 
-    pub fn visitors(visitors: Vec<(Vec<AstParentKind>, Box<dyn VisitorFactory>)>) -> Self {
+    pub fn visitors(visitors: Vec<(Vec<AstParentKind>, Box<dyn AstModifier>)>) -> Self {
         #[cfg(debug_assertions)]
         for (path, _) in visitors.iter() {
             if path.is_empty() {
@@ -113,10 +113,6 @@ impl CodeGenerationHoistedStmt {
     }
 }
 
-pub trait VisitorFactory: Send + Sync {
-    fn create<'a>(&'a self) -> Box<dyn AstModifier + 'a>;
-}
-
 pub trait PassFactory: Send + Sync {
     fn create<'a>(&'a self) -> Box<dyn Pass + Send + Sync + 'a>;
 }
@@ -279,16 +275,8 @@ macro_rules! create_visitor {
             $name: T,
         }
 
-        impl<T: Fn(&mut swc_core::ecma::ast::$ty) + Send + Sync> $crate::code_gen::VisitorFactory
-            for Box<Visitor<T>>
-        {
-            fn create<'a>(&'a self) -> Box<dyn $crate::code_gen::AstModifier + 'a> {
-                Box::new(&**self)
-            }
-        }
-
-        impl<'a, T: Fn(&mut swc_core::ecma::ast::$ty) + Send + Sync> $crate::code_gen::AstModifier
-            for &'a Visitor<T>
+        impl<T: Fn(&mut swc_core::ecma::ast::$ty) + Send + Sync> $crate::code_gen::AstModifier
+            for Visitor<T>
         {
             fn $name(&self, $arg: &mut swc_core::ecma::ast::$ty) {
                 (self.$name)($arg);
@@ -303,9 +291,9 @@ macro_rules! create_visitor {
 
             (
                 $ast_path,
-                Box::new(Box::new(Visitor {
+                Box::new(Visitor {
                     $name: move |$arg: &mut swc_core::ecma::ast::$ty| $b,
-                })) as Box<dyn $crate::code_gen::VisitorFactory>,
+                }) as Box<dyn $crate::code_gen::AstModifier>,
             )
         }
     }};

filename: turbopack/crates/turbopack-ecmascript/src/path_visitor.rs
patch: @@ -8,26 +8,26 @@ use swc_core::{
     },
 };
 
-use crate::code_gen::{ModifiableAst, VisitorFactory};
+use crate::code_gen::{AstModifier, ModifiableAst};
 
 pub type AstPath = Vec<AstParentKind>;
 
 // Invariant: Each [AstPath] in `visitors` contains a value at position `index`.
 pub struct ApplyVisitors<'a, 'b> {
     /// `VisitMut` should be shallow. In other words, it should not visit
     /// children of the node.
-    visitors: Cow<'b, [(&'a AstPath, &'a dyn VisitorFactory)]>,
+    visitors: Cow<'b, [(&'a AstPath, &'a dyn AstModifier)]>,
 
     index: usize,
 }
 
 /// Do two binary searches to find the sub-slice that has `path[index] == kind`.
 /// Returns None if no item matches that. `visitors` need to be sorted by path.
 fn find_range<'a, 'b>(
-    visitors: &'b [(&'a AstPath, &'a dyn VisitorFactory)],
+    visitors: &'b [(&'a AstPath, &'a dyn AstModifier)],
     kind: &AstParentKind,
     index: usize,
-) -> Option<&'b [(&'a AstPath, &'a dyn VisitorFactory)]> {
+) -> Option<&'b [(&'a AstPath, &'a dyn AstModifier)]> {
     // Precondition: visitors is never empty
     if visitors.first().unwrap().0[index] > *kind || visitors.last().unwrap().0[index] < *kind {
         // Fast path: If ast path of the first visitor is already out of range, then we
@@ -67,7 +67,7 @@ fn find_range<'a, 'b>(
 
 impl<'a> ApplyVisitors<'a, '_> {
     /// `visitors` must have an non-empty [AstPath].
-    pub fn new(mut visitors: Vec<(&'a AstPath, &'a dyn VisitorFactory)>) -> Self {
+    pub fn new(mut visitors: Vec<(&'a AstPath, &'a dyn AstModifier)>) -> Self {
         assert!(!visitors.is_empty());
         visitors.sort_by_key(|(path, _)| *path);
         Self {
@@ -109,7 +109,7 @@ impl<'a> ApplyVisitors<'a, '_> {
                         );
                     }
                     for (_, visitor) in visitors[..nested_visitors_start].iter() {
-                        n.modify(&*visitor.create());
+                        n.modify(&**visitor);
                     }
                     return;
                 } else {
@@ -172,8 +172,7 @@ mod tests {
         testing::run_test,
     };
 
-    use super::{ApplyVisitors, VisitorFactory};
-    use crate::code_gen::AstModifier;
+    use super::{ApplyVisitors, AstModifier};
 
     fn parse(fm: &SourceFile) -> Module {
         let mut m = parse_file_as_module(
@@ -198,20 +197,14 @@ mod tests {
         to: &'a str,
     }
 
-    impl VisitorFactory for Box<StrReplacer<'_>> {
-        fn create<'a>(&'a self) -> Box<dyn AstModifier + 'a> {
-            Box::new(&**self)
-        }
-    }
-
-    impl AstModifier for &'_ StrReplacer<'_> {
+    impl AstModifier for StrReplacer<'_> {
         fn visit_mut_str(&self, s: &mut Str) {
             s.value = s.value.replace(self.from, self.to).into();
             s.raw = None;
         }
     }
 
-    fn replacer(from: &'static str, to: &'static str) -> impl VisitorFactory {
+    fn replacer(from: &'static str, to: &'static str) -> Box<dyn AstModifier + 'static> {
         Box::new(StrReplacer { from, to })
     }
 
@@ -264,7 +257,7 @@ mod tests {
 
                 let mut m = m.clone();
                 m.visit_mut_with_ast_path(
-                    &mut ApplyVisitors::new(vec![(&path, &bar_replacer)]),
+                    &mut ApplyVisitors::new(vec![(&path, &*bar_replacer)]),
                     &mut Default::default(),
                 );
 
@@ -289,7 +282,7 @@ mod tests {
 
                 let mut m = m.clone();
                 m.visit_mut_with_ast_path(
-                    &mut ApplyVisitors::new(vec![(&wrong_path, &bar_replacer)]),
+                    &mut ApplyVisitors::new(vec![(&wrong_path, &*bar_replacer)]),
                     &mut Default::default(),
                 );