perf: reduce string allocations#10423
Conversation
How to use the Graphite Merge QueueAdd the label graphite: merge-when-ready to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR continues the post-oxc-constructor migration performance work by reusing already-arena-backed Str<'ast> / Ident<'ast> values when building new AST nodes, avoiding redundant &str conversions and re-allocation (and for identifiers, avoiding re-hashing in some paths).
Changes:
- Reuse existing
Str/Identvalues when constructingIdentifierName,StringLiteral, and identifier expressions in the HMR AST finalizer. - Simplify property-key construction in the module finalizer by using
PropertyKey::new_static_identifier(...)directly (removingIntoIn/manual boxing/allocation patterns).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/rolldown/src/module_finalizers/impl_visit_mut.rs | Switches assignment-target property key construction to reuse existing arena strings via PropertyKey::new_static_identifier. |
| crates/rolldown/src/hmr/hmr_ast_finalizer.rs | Reuses existing Str/Ident values for member-expression properties, string literals, and exported identifier expressions to reduce arena allocations. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
overlookmotel
changed the title
9d0b7c0 to
f677ba7
Compare
c583ea9 to
6466383
Compare
Merge activity
|
Follow-on after #10018, continuation of #10420 and #10421. Where we already have a `Str<'ast>` or `Ident<'ast>`, avoid converting it into a `&str` and then allocating it back into arena again to convert back to `Str` or `Ident`. Just re-use the existing `Str` / `Ident`. This avoids unnecessary arena allocations, and in the case of `Ident`, it avoids re-hashing the string too.
f677ba7 to
89ec37d
Compare
6466383 to
55729bb
Compare
Follow-on after #10018, continuation of #10420 and #10421.
Where we already have a
Str<'ast>orIdent<'ast>, avoid converting it into a&strand then allocating it back into arena again to convert back toStrorIdent. Just re-use the existingStr/Ident. This avoids unnecessary arena allocations, and in the case ofIdent, it avoids re-hashing the string too.