refactor(isolated_declarations): do not store temp values in arena #9733
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
github-actions
bot
added
A-isolated-declarations
CodSpeed Performance ReportMerging #9733 will not alter performanceComparing Summary
|
Merge activity
|
…9733) `stmts` here is an `ArenaVec<&Statement>` (`&Statement` not `Statement`). This is a temporary collection, and doesn't end up in the AST, so we shouldn't store it in the arena. Use a `std::vec::Vec` instead of `oxc_allocator::Vec`. This should also remove some of the lifetime problems we have in #9656. This change may cause a small perf hit, but in my view it's still a good change. If we want to get a speed boost, a better solution would be to have a temporary "scratch space" arena which we can allocate *all* temporary values into. This would likely give us a sizeable speed boost across many parts of Oxc (oxc-project/backlog#121).
graphite-app
bot
force-pushed
the
03-12-refactor_isolated_declarations_do_not_store_temp_values_in_arena
branch
from
592ed29 to
a61a50b
Compare
|
I've taken off the merge label in hope of stopping Graphite - but maybe it's unstoppable! Would like to see the benchmarks first ideally. |
graphite-app
bot
deleted the
03-12-refactor_isolated_declarations_do_not_store_temp_values_in_arena
branch
Only 0.2ms dropped 
|
|
-0.3% perf hit. That's no so bad. |
stmtshere is anArenaVec<&Statement>(&StatementnotStatement). This is a temporary collection, and doesn't end up in the AST, so we shouldn't store it in the arena.Use a
std::vec::Vecinstead ofoxc_allocator::Vec.This should also remove some of the lifetime problems we have in #9656.
This change may cause a small perf hit, but in my view it's still a good change. If we want to get a speed boost, a better solution would be to have a temporary "scratch space" arena which we can allocate all temporary values into. This would likely give us a sizeable speed boost across many parts of Oxc (oxc-project/backlog#121).