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: Arena based ast #9805

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

CPunisher
Copy link
Contributor

@CPunisher CPunisher commented Dec 19, 2024

How to do it

The process of refactoring is very mechanical and labor intensive:

  1. Declare lifetimes for all ast structs and other related structs and traits.
  2. Create all ast nodes using bumpalo allocator.
  3. Fix lots of compilation errors.

Effectiveness

I have almost finished swc_ecma_ast, swc_ecma_parser and swc_ecma_visit crates. You can run the benchmark with cargo bench -p swc_ecma_parser --bench arena_compare:

Parser VisitMut
Swc baseline 12.963ms 424.98us
Swc arena 11.881ms 374.05us
Oxc 4.3084ms 372.51us

Existing Barriers

When I was refactoring the parser, I found the following steps to be very mechanical and cumbersome. I think it will greatly reduce the developer enthusiasm for refactoring codebase and downstream applications.

  1. Unavailable: Ast conversion by bridging Into.
  2. Unavailable: Pattern match with struct destruction
  3. Unavailable: AstNode::default()
  4. Unavailable: Ast deserialization.
  5. Wrap many ast constructions in Box::new_in(.., self.alloc)

Copy link

codspeed-hq bot commented Dec 19, 2024

CodSpeed Performance Report

Merging #9805 will degrade performances by 5.97%

Comparing CPunisher:perf/arena (fad24ea) with main (c81be2e)

Summary

❌ 1 regressions
✅ 193 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main CPunisher:perf/arena Change
es/visitor/base-perf/boxing_boxed_clone 2.3 µs 2.4 µs -5.97%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant