perf(formatter): do not write elements to the flat buffer if the layout is multline#16630
Conversation
github-actions
bot
added
A-formatter
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. |
CodSpeed Performance ReportMerging #16630 will improve performances by 3.97%Comparing Summary
Benchmarks breakdown
Footnotes
|
Dunqing
force-pushed
the
12-09-perf_formatter_do_not_write_elements_to_the_flat_buffer_if_the_layout_is_multline
branch
3 times, most recently
from
02172b8 to
b3581c8
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes the JSX child list formatter by avoiding unnecessary writes to the flat buffer when the layout is known to be multiline from the start. The optimization prevents wasteful allocations and format computations by initializing the FlatBuilder in a disabled state when force_multiline is true, rather than writing elements and then disabling it later.
Key changes:
- Modified
FlatBuilder::new()to accept adisabledparameter for early initialization - Added conditional logic to skip flat buffer writes when a child element will break
- Reordered initialization to compute
force_multilinebefore creating theFlatBuilder
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge activity
|
…ut is multline (#16630) https://github.com/oxc-project/oxc/blob/c17b3d4b471d5404f67b6bb6a55921d173014318/crates/oxc_formatter/src/write/jsx/child_list.rs#L49-L50 If `force_multiline` is true, that means the flat buffer will never be used, so we should pass a flag to disable the flat buffer at the start, rather than disabling it after the first child was written in the flat buffer Disabling here is too late; a child has already been written in the flat buffer, which causes a little bit of a performance hit. https://github.com/oxc-project/oxc/blob/b3581c85f710f79de1a1dfe9af731a7b2edf1b88/crates/oxc_formatter/src/write/jsx/child_list.rs#L328-L331
graphite-app
bot
force-pushed
the
12-09-perf_formatter_do_not_write_elements_to_the_flat_buffer_if_the_layout_is_multline
branch
from
b3581c8 to
4843475
Compare
graphite-app
bot
deleted the
12-09-perf_formatter_do_not_write_elements_to_the_flat_buffer_if_the_layout_is_multline
branch
…ut is multline (#16630) https://github.com/oxc-project/oxc/blob/c17b3d4b471d5404f67b6bb6a55921d173014318/crates/oxc_formatter/src/write/jsx/child_list.rs#L49-L50 If `force_multiline` is true, that means the flat buffer will never be used, so we should pass a flag to disable the flat buffer at the start, rather than disabling it after the first child was written in the flat buffer Disabling here is too late; a child has already been written in the flat buffer, which causes a little bit of a performance hit. https://github.com/oxc-project/oxc/blob/b3581c85f710f79de1a1dfe9af731a7b2edf1b88/crates/oxc_formatter/src/write/jsx/child_list.rs#L328-L331
…ut is multline (oxc-project#16630) https://github.com/oxc-project/oxc/blob/c17b3d4b471d5404f67b6bb6a55921d173014318/crates/oxc_formatter/src/write/jsx/child_list.rs#L49-L50 If `force_multiline` is true, that means the flat buffer will never be used, so we should pass a flag to disable the flat buffer at the start, rather than disabling it after the first child was written in the flat buffer Disabling here is too late; a child has already been written in the flat buffer, which causes a little bit of a performance hit. https://github.com/oxc-project/oxc/blob/b3581c85f710f79de1a1dfe9af731a7b2edf1b88/crates/oxc_formatter/src/write/jsx/child_list.rs#L328-L331
oxc/crates/oxc_formatter/src/write/jsx/child_list.rs
Lines 49 to 50 in c17b3d4
If
force_multilineis true, that means the flat buffer will never be used, so we should pass a flag to disable the flat buffer at the start, rather than disabling it after the first child was written in the flat bufferDisabling here is too late; a child has already been written in the flat buffer, which causes a little bit of a performance hit.
oxc/crates/oxc_formatter/src/write/jsx/child_list.rs
Lines 328 to 331 in b3581c8