Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hardfist opened this issue Nov 18, 2024 · 3 comments
Open

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

hardfist opened this issue Nov 18, 2024 · 3 comments
Assignees
Labels
team The issue/pr is created by the member of Rspack.

Comments

@hardfist
Copy link
Contributor

hardfist commented Nov 18, 2024

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

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

source: original_source.clone(),

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Nov 18, 2024
@GiveMe-A-Name GiveMe-A-Name self-assigned this Nov 18, 2024
@GiveMe-A-Name
Copy link
Member

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
Copy link
Collaborator

h-a-n-a commented Nov 18, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

No branches or pull requests

3 participants