perf(transformer/refresh): use `take/take_in` instead of `drain` by Dunqing · Pull Request #10656 · oxc-project/oxc

Dunqing · 2025-04-28T08:58:21Z

Calling take is cheaper than calling drain. These cases don't need to reuse the Vec after it drained, so we can replace them with take, which gives a tiny performance improvement.

Dunqing · 2025-04-28T08:58:37Z

perf(allocator/vec2): align min amortized cap size with std #9857
feat(allocator/vec2): align RawVec::reserve with standard library implementation #10701
perf(allocator/vec2): replace self.reserve(1) calls with self.grow_one() for better efficiency #9856
feat(allocator/vec2): add specialized grow_one method #9855
perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly instead of calling realloc #10686
perf(allocator/vec2): resolve performance regression for extend by marking reserve as #[cold] and #[inline(never)] #10675
feat(allocator/vec2): introduce extend_desugared method as extend internal implementation #10670
perf(transformer/refresh): use take/take_in instead of drain #10656 👈 (View in Graphite)
perf(transformer): optimize inserting var/let statements #10654
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

0-merge - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

codspeed-hq · 2025-04-28T09:11:28Z

CodSpeed Instrumentation Performance Report

Merging #10656 will not alter performance

_{Comparing 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain (7d8efd8) with main (a3ada34)}

Summary

✅ 36 untouched benchmarks

overlookmotel

Good spot. Though I'm not actually sure how much take_in gains over drain(..) here. The code for drain contains more instructions, but in the case of drain(..) compiler may be able to optimize most of them out. Maybe. That's just guesswork.

I suspect the perf gain in this PR is more due to:

- let mut new_statements = ctx.ast.vec_with_capacity(program.body.len());
+ let mut new_statements = ctx.ast.vec_with_capacity(program.body.len() * 2);

...so new_statements will never need to grow and reallocate during the loop.

I can see another couple of things. I'll do a follow-up PR.

overlookmotel · 2025-04-29T16:29:35Z

Merge activity

Apr 29, 12:29 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 29, 12:39 PM EDT: overlookmotel added this pull request to the Graphite merge queue.
Apr 29, 5:00 PM UTC: The Graphite merge queue couldn't merge this PR because it timed out.
Apr 29, 1:05 PM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 29, 8:08 PM EDT: overlookmotel added this pull request to the Graphite merge queue.
Apr 29, 8:09 PM EDT: Merged by the Graphite merge queue.

) Calling `take` is cheaper than calling `drain`. These cases don't need to reuse the `Vec` after it drained, so we can replace them with `take`, which gives a tiny performance improvement. <img width="633" alt="image" src="https://github.com/user-attachments/assets/a2a65957-09a0-47a5-9b9f-eaec576220c1" />

…0712) Follow-on after #10656. There's no need to consume `self.registrations` because we only need a reference to `binding` (`BoundIdentifier`), and the `persistent_id` is an `Atom`, which is `Copy`. Just iterate over `&self.registrations`.

Follow-on after #10656. Remove the temporary `Vec` `new_statements`. Append new statements directly to `program.body` instead. We get around the fact that the `VariableDeclaration` has to be inserted first by initially pushing a `VariableDeclaration` with no declarators, and then setting its declarators at the end.

Dunqing mentioned this pull request Apr 28, 2025

perf(transformer): optimize inserting var/let statements #10654

Merged

github-actions bot added A-transformer Area - Transformer / Transpiler C-performance Category - Solution not expected to change functional behavior, only performance labels Apr 28, 2025

Dunqing changed the base branch from 04-28-perf_transformer_optimize_inserting_var_let_statements to graphite-base/10656 April 28, 2025 14:23

Dunqing force-pushed the graphite-base/10656 branch from 0479363 to 68620ef Compare April 28, 2025 14:30

Dunqing force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from aecfae5 to 3648471 Compare April 28, 2025 14:30

Dunqing changed the base branch from graphite-base/10656 to 04-28-perf_transformer_optimize_inserting_var_let_statements April 28, 2025 14:30

Dunqing changed the base branch from 04-28-perf_transformer_optimize_inserting_var_let_statements to graphite-base/10656 April 29, 2025 02:25

Dunqing force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from 3648471 to c01878f Compare April 29, 2025 02:32

Dunqing force-pushed the graphite-base/10656 branch from 68620ef to b44d2a9 Compare April 29, 2025 02:32

Dunqing changed the base branch from graphite-base/10656 to 04-28-perf_transformer_optimize_inserting_var_let_statements April 29, 2025 02:32

Dunqing mentioned this pull request Apr 29, 2025

perf(allocator/vec2): calling Bump::grow or Bump::shrink at the call site directly instead of calling realloc #10686

Merged

Dunqing marked this pull request as ready for review April 29, 2025 09:31

Dunqing requested a review from overlookmotel as a code owner April 29, 2025 09:31

Dunqing mentioned this pull request Apr 29, 2025

feat(allocator/vec2): align RawVec::reserve with standard library implementation #10701

Merged

graphite-app bot changed the base branch from 04-28-perf_transformer_optimize_inserting_var_let_statements to graphite-base/10656 April 29, 2025 16:01

graphite-app bot force-pushed the graphite-base/10656 branch from b44d2a9 to 91df9d4 Compare April 29, 2025 16:09

graphite-app bot force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from c01878f to 58652a1 Compare April 29, 2025 16:09

graphite-app bot changed the base branch from graphite-base/10656 to main April 29, 2025 16:10

graphite-app bot force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from 58652a1 to 25f8411 Compare April 29, 2025 16:10

overlookmotel approved these changes Apr 29, 2025

View reviewed changes

overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 29, 2025

graphite-app bot force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from 25f8411 to 04db587 Compare April 29, 2025 16:40

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 29, 2025

overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 29, 2025

Dunqing force-pushed the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch from 04db587 to 7d8efd8 Compare April 30, 2025 00:01

graphite-app bot merged commit 7d8efd8 into main Apr 30, 2025
26 checks passed

graphite-app bot deleted the 04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain branch April 30, 2025 00:09

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 30, 2025

Dunqing mentioned this pull request Apr 30, 2025

perf(allocator/vec2): align min amortized cap size with std #9857

Merged

overlookmotel mentioned this pull request Apr 30, 2025

refactor(transformer/refresh): do not unnecessarily consume Vec #10712

Merged

overlookmotel mentioned this pull request Apr 30, 2025

perf(transformer/refresh): remove temp Vec #10713

Merged

oxc-bot mentioned this pull request May 3, 2025

release(crates): v0.68.0 #10777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(transformer/refresh): use `take/take_in` instead of `drain`#10656

perf(transformer/refresh): use `take/take_in` instead of `drain`#10656
graphite-app[bot] merged 1 commit intomainfrom
04-28-perf_transformer_refresh_use_take_take_in_instead_of_drain

Dunqing commented Apr 28, 2025 •

edited

Loading

Uh oh!

Dunqing commented Apr 28, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Apr 28, 2025 •

edited

Loading

Uh oh!

overlookmotel left a comment •

edited

Loading

Uh oh!

overlookmotel commented Apr 29, 2025 •

edited by graphite-app bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Dunqing commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dunqing commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #10656 will not alter performance

Summary

Uh oh!

overlookmotel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overlookmotel commented Apr 29, 2025 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dunqing commented Apr 28, 2025 •

edited

Loading

Dunqing commented Apr 28, 2025 •

edited

Loading

codspeed-hq bot commented Apr 28, 2025 •

edited

Loading

overlookmotel left a comment •

edited

Loading

overlookmotel commented Apr 29, 2025 •

edited by graphite-app bot

Loading