Perf: Reuse AST passed from bultin:swc_loader to avoid double parse

[hardfist]

span is not valid after transformation

currrently Rspack will do double parse for all file processed by builtin:swc-loader, the reason Rspack do double parse is caused by swc transform will make the origin span invalid which cause it's bad for following dependency generation

• before transformed by builtin:swc-loader

import { b }  from './b';
const  a = b;  // now AstNode('b') span is 37-38

• after transformed by builtin:swc-loader

import { b }  from './b';
var a = b; // the span in ast is still 37-38 but after codegen the valid span is actually 34-35

span can be fixed by srcmapbuf generated by codegen

so the span in ASTNode is not valid after transformation, but we can fix the span use src_map generated by emitter, the src_map contains the right mapping from span in ast to span after codegen

rspack/crates/rspack_plugin_javascript/src/ast/stringify.rs

Lines 79 to 84 in 9112e35

	let mut wr = Box::new(text_writer::JsWriter::new(
	source_map.clone(),
	"\n",
	&mut buf,
	source_map_config.enable.then_some(&mut src_map_buf),
	)) as Box<dyn WriteJs>;

we fix ast span using visit_mut_span in SpanFixerVisitor

struct SpanFixerVisitor {
   src_map_buf: SrcMapBuf
};
impl VisitMut for SpanFixerVisitor {
   fn visit_mut_span(&mut self, span: &mut Span){
             let codegen_span = self.src_map_buf.get(span);
             span.start = codegen_span.start;
             span.end = codegen_span.end;
   }
}

ast invalidation

we should mark the ast dirty | invalid, if the code is modified by following loader

reuse span in module.build

if the AST contains the valid span, then parse can reuse ast passing from builtin:swc-loader, no need to do double parse in module.build

rspack/crates/rspack_core/src/normal_module.rs

Line 542 in 9112e35

source: original_source.clone(),

[GiveMe-A-Name]

Except fix the span, we need pass ast from swc_loader to JavaScriptParserAndGenerator.
We can use additional_data: anymap::Map<dyn CloneAny + Send + Sync> to pass ast.

// swc_loader
fn run(loader_context) {
 ....
 let mut additional_data = Default::default();
 additional_data.insert(ast);
 
 loader_context.finish((code, map, additional_data))
}

then get ast from additional_data

// parser_and_generator
fn parse(parse_context) {
   let ParseContext { additional_data } = parse_context;

   let ast = additional_data.get::<SwcAst>();

   ....
}

[h-a-n-a]

We can use additional_data: anymap::Map<dyn CloneAny + Send + Sync> to pass ast.

Voting for this solution.

Given the status-quo, AdditionalData is replaced by each loader, so there's no need to introduce the mental model to even invalidating the AST.

[GiveMe-A-Name]

Actually, we can't get mapping that from source span to transformed span using src_map_buf.
The src_map_buf is used for generate source_map, It only maps a portion of the span's start or end positions to the corresponding source line and column numbers.
We need other way.

[GiveMe-A-Name]

In swc transformer, it would create some default span node in ast.
For example: if we want transform jsx that set react.runtime as automatic. It would set some zero-span node.

const element = <div></div>;

After transform

// ast
Script(Script { span: 1..29, body: [Decl(Var(VarDecl { span: 0..0, ctxt: #0, kind: "var", declare: false, decls: [VarDeclarator { span: 0..0, name: Ident(BindingIdent { id: Ident { span: 0..0, ctxt: #0, sym: "_require", optional: false }, type_ann: None }), init: Some(Call(CallExpr { span: 0..0, ctxt: #0, callee: Expr(Ident(Ident { span: 0..0, ctxt: #0, sym: "require", optional: false })), args: [ExprOrSpread { spread: None, expr: Lit(Str(Str { span: 0..0, value: "react/jsx-runtime", raw: None })) }], type_args: None })), definite: false }, VarDeclarator { span: 0..0, name: Ident(BindingIdent { id: Ident { span: 0..0, ctxt: #0, sym: "_jsx", optional: false }, type_ann: None }), init: Some(Member(MemberExpr { span: 0..0, obj: Ident(Ident { span: 0..0, ctxt: #0, sym: "_require", optional: false }), prop: Ident(IdentName { span: 0..0, sym: "jsx" }) })), definite: false }] })), Decl(Var(VarDecl { span: 1..29, ctxt: #0, kind: "var", declare: false, decls: [VarDeclarator { span: 7..28, name: Ident(BindingIdent { id: Ident { span: 7..14, ctxt: #0, sym: "element", optional: false }, type_ann: None }), init: Some(Call(CallExpr { span: 17..28, ctxt: #0, callee: Expr(Ident(Ident { span: 0..0, ctxt: #0, sym: "_jsx", optional: false })), args: [ExprOrSpread { spread: None, expr: Lit(Str(Str { span: 18..21, value: "div", raw: None })) }, ExprOrSpread { spread: None, expr: Object(ObjectLit { span: 0..0, props: [] }) }], type_args: None })), definite: false }] }))], shebang: None })

var _require = require("react/jsx-runtime"), _jsx = _require.jsx;
var element = _jsx("div", {});

So we may not get map original_node_span to gen_node_span only using records way.
I will try anther ways.

[JSerFeng]

It would set some zero-span node.

Is this a bug of swc ?

[GiveMe-A-Name]

It would set some zero-span node.

Is this a bug of swc ?

I don't think, swc can't get the span information util ast generate actual code.

[JSerFeng]

Oh I see, it uses DUMMY_SP everywhere to generate extra AST during transform

[GiveMe-A-Name]

I do some simple bench test. It's not too optimistic to reuse ast now.
I run rspack-ecosystem-benchmark first job 10000_development-mode in my macbook(M1 Pro 32G).
The result bellow:

Case	median	mean
Not reuse ast(parse twice)	10.6s	9.96s
Reuse ast and fix span info	9.37s	9.9s
Only reuse ast not fix span info	9.11s	9.02s

I guess that fix span info need rewrite almost all ast node span status. It's will cost many time.
I do not think we should reuse ast now. We like webpack take transformer as loader.
We handle the js code in string layer. In the future, if we can handle js code in ast layer, reuse ast is more useful.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still affecting you, please leave any comment (for example, "bump"). We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[hardfist]

bump