• feat(rust): ensure span uniqueness of oxc AST #1927 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @hyf0 and the rest of your teammates on Graphite

✅ Deploy Preview for rolldown-rs canceled.

Benchmarks Rust

• target: main(3ea6797)
• pr: 08-10-feat_rust_ensure_span_uniqueness_of_oxc_ast(d59dcdd)

group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.00     52.1±0.55ms        ? ?/sec    1.01     52.9±2.82ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     74.8±0.73ms        ? ?/sec    1.02     76.3±1.71ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.00     92.3±2.90ms        ? ?/sec    1.00     92.7±1.53ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     59.8±1.19ms        ? ?/sec    1.00     59.7±0.89ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00     99.7±0.82ms        ? ?/sec    1.00    100.1±1.12ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.00    184.2±6.02ms        ? ?/sec    1.00    184.6±4.37ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.01    226.1±5.03ms        ? ?/sec    1.00    224.6±4.22ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.00    115.6±1.83ms        ? ?/sec    1.00    115.3±1.54ms        ? ?/sec
bundle/bundle@threejs                                               1.03     32.9±1.02ms        ? ?/sec    1.00     31.8±0.52ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.04     79.6±1.26ms        ? ?/sec    1.00     76.8±1.28ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.03     95.4±1.65ms        ? ?/sec    1.00     92.5±1.54ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.00     41.1±0.54ms        ? ?/sec    1.02     42.0±0.76ms        ? ?/sec
bundle/bundle@threejs10x                                            1.01    347.9±3.10ms        ? ?/sec    1.00    345.7±3.16ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.02    944.3±5.30ms        ? ?/sec    1.00    927.0±4.93ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00  1179.1±14.18ms        ? ?/sec    1.00  1176.0±14.97ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.00    415.0±2.70ms        ? ?/sec    1.00    414.6±3.73ms        ? ?/sec
remapping/remapping                                                 1.00     32.4±0.11ms        ? ?/sec    1.02     33.1±0.19ms        ? ?/sec
remapping/render-chunk-remapping                                    1.00     84.6±0.39ms        ? ?/sec    1.01     85.6±0.86ms        ? ?/sec
scan/scan@rome-ts                                                   1.01     80.6±0.57ms        ? ?/sec    1.00     80.0±0.90ms        ? ?/sec
scan/scan@threejs                                                   1.01     24.8±0.14ms        ? ?/sec    1.00     24.5±0.52ms        ? ?/sec
scan/scan@threejs10x                                                1.01    248.8±2.61ms        ? ?/sec    1.00    245.4±1.85ms        ? ?/sec

A few thoughts:

1. If you're only checking spans of ModuleDeclarations, you don't need a full AST traversal. They can only be at top level, so you could just iterate through program.statements.

2. If you're only looking for duplicate "fake" spans, as start and end of "fake" spans are always equal, visited_spans: FxHashSet<Span> can be replaced with visited_span_starts: FxHashSet<u32> and ensure_uniqueness can be:

fn ensure_uniqueness(&mut self, span: &mut Span) {
    // `span.start == span.end` will be false for all real spans,
    // and equality check is much cheaper than hashmap lookup
    if span.start == span.end && self.visited_span_starts.contains(span.start) {
        *span = self.generate_unique_span();
        self.visited_span_starts.insert(span.start);
    }
}

3. Please see my comment on Introduce visit_span on VisitMut and Visit oxc-project/oxc#4799 as to why using visit_span is probably a bad idea.

If you're only checking spans of ModuleDeclarations. you don't need a full AST traversal.

There are other parts need this too, including MemberExpression, so we need a almost full AST traversal. I didn't do it because they aren't required for supporting #1926.

If you're only looking for duplicate "fake" spans, as start and end of "fake" spans are always equal, visited_spans: FxHashSet can be replaced with visited_span_starts: FxHashSet and ensure_uniqueness can be:

Nope. We need to make sure all AST nodes don't have duplicate span.

When I say "all AST nodes" it means almost all AST nodes, since we care about MemberExpression. A full AST traversal seems unavoidable.

Ah ha. Thanks for explaining. What MemberExpressions are you looking for? I can see you'd need to also track import() and require(), but I can't imagine why MemberExpressions.

Nope. We need to make sure all AST nodes don't have duplicate span.

Out of interest, how is it possible for 2 AST nodes to have same span if they are "real" spans i.e. from Oxc parser? Is this something that can happen when you combine two ASTs into one?

Sorry for "noob" questions. I have very little knowledge of Rolldown, but would like to gain some!

Ah ha. Thanks for explaining. What MemberExpressions are you looking for? I can see you'd need to also track import() and require(), but I can't imagine why MemberExpressions.

Nope. We need to make sure all AST nodes don't have duplicate span.

Out of interest, how is it possible for 2 AST nodes to have same span if they are "real" spans i.e. from Oxc parser? Is this something that can happen when you combine two ASTs into one?

Sorry for "noob" questions. I have very little knowledge of Rolldown, but would like to gain some!

After oxc transform (or any custom transform), the newly added Ast will have SPAN which are (0, 0). They are both same

Ah ha. Thanks for explaining. What MemberExpressions are you looking for? I can see you'd need to also track import() and require(), but I can't imagine why MemberExpressions.

Nope. We need to make sure all AST nodes don't have duplicate span.

Out of interest, how is it possible for 2 AST nodes to have same span if they are "real" spans i.e. from Oxc parser? Is this something that can happen when you combine two ASTs into one?

Sorry for "noob" questions. I have very little knowledge of Rolldown, but would like to gain some!

#1818 (comment). We will resolve MemberExpression stored in the first visit to a symbol and rewrite the MemberExpression to that Symbol. We rely on Span to identify the MemberExpression if they are the same in two visit pass.