Inline fits_element in formatter#26429
Merged
Merged
Conversation
|
Merging this PR will improve performance by 4.94%
Performance Changes
Tip Curious why this is faster? Use the CodSpeed MCP and ask your agent. Comparing Footnotes
|
MichaReiser
approved these changes
Jun 27, 2026
MichaReiser
left a comment
Member
There was a problem hiding this comment.
Nice (ignoring the title)
charliermarsh
marked this pull request as ready for review
June 27, 2026 16:23
fits_element in formatter
This was referenced Jun 30, 2026
graphite-app Bot
pushed a commit
to oxc-project/oxc
that referenced
this pull request
Jun 30, 2026
## Summary - inline the formatter-core `FitsMeasurer::fits_element` dispatcher - mirrors the applicable hot-loop optimization from astral-sh/ruff#26429 ## Local benchmark Command: ```sh cargo bench -p oxc_benchmark --bench formatter --no-default-features --features compiler -- --sample-size 20 --warm-up-time 1 --measurement-time 3 ``` Compared `main` (`d05773604f`) against this branch (`356e65db1c`) with Criterion saved baselines. Median times from a single local run: | fixture | main median | PR median | delta | | --- | ---: | ---: | ---: | | `RadixUIAdoptionSection.jsx` | 41.82 us | 39.69 us | -5.09% | | `errors.ts` | 60.51 us | 56.20 us | -7.12% | | `Search.tsx` | 249.52 us | 239.56 us | -3.99% | | `core.js` | 230.19 us | 221.52 us | -3.77% | | `next.ts` | 360.07 us | 348.98 us | -3.08% | | `index.tsx` | 712.25 us | 673.43 us | -5.45% | | `handle-comments.js` | 394.35 us | 376.84 us | -4.44% | | `types.ts` | 1.393 ms | 1.304 ms | -6.42% | | `App.tsx` | 8.928 ms | 8.156 ms | -8.64% | Geomean across these formatter fixtures: -5.35% median runtime. Treat as directional local data; CI/CodSpeed is the source of truth.
charliermarsh
added a commit
that referenced
this pull request
Jul 2, 2026
## Summary The formatter calls `Printer::print_element` for every rendered format element and `Printer::print_text` for every text element. LLVM leaves both private methods out of line even when they are marked `#[inline]` and the release profile uses fat LTO, so these hot loops retain out-of-line calls. This marks both functions as `#[inline(always)]`, allowing the printing loop and its common text paths to optimize together. It follows the same general source-level inlining approach as #26429 and closed #26473, but targets the printing pass rather than fitting or comment extraction. The methods have a small, fixed set of call sites, and the release binary's `.text` grows by 11,264 bytes (0.36%). ## Performance To test whether ordinary `#[inline]` is sufficient and whether LTO changes the result, I built the exact base with no annotation, `#[inline]`, and `#[inline(always)]` under both the profiling profile (LTO disabled) and the release profile (fat LTO). Plain `#[inline]` neither removed the out-of-line symbols nor changed `.text`, and CodSpeed reported 0.0% in both profiles. Forced inlining removed both symbols and preserved the improvement under fat LTO: | Profile | Annotation | Overall impact | `.text` delta | | --- | --- | ---: | ---: | | Profiling (no LTO) | `#[inline]` | 0.0% | 0 bytes | | Profiling (no LTO) | `#[inline(always)]` | +6.2% | +11,152 bytes (+0.35%) | | Release (fat LTO) | `#[inline]` | 0.0% | 0 bytes | | Release (fat LTO) | `#[inline(always)]` | +6.0% | +11,264 bytes (+0.36%) | The release/fat-LTO comparison on the existing non-microbenchmark formatter suite reports: | Benchmark | Base | Head | Speedup | | --- | ---: | ---: | ---: | | `formatter[large/dataset.py]` | 7.53 ms | 7.02 ms | 7.3% | | `formatter[pydantic/types.py]` | 2.99 ms | 2.79 ms | 7.2% | | `formatter[numpy/ctypeslib.py]` | 1.57 ms | 1.48 ms | 5.9% | | `formatter[unicode/pypinyin.py]` | 580.30 µs | 550.17 µs | 5.5% | | `formatter[numpy/globals.py]` | 217.95 µs | 209.04 µs | 4.3% |
camc314
added a commit
to oxc-project/oxc
that referenced
this pull request
Jul 3, 2026
## Summary - inline the formatter-core `FitsMeasurer::fits_element` dispatcher - mirrors the applicable hot-loop optimization from astral-sh/ruff#26429 ## Local benchmark Command: ```sh cargo bench -p oxc_benchmark --bench formatter --no-default-features --features compiler -- --sample-size 20 --warm-up-time 1 --measurement-time 3 ``` Compared `main` (`d05773604f`) against this branch (`356e65db1c`) with Criterion saved baselines. Median times from a single local run: | fixture | main median | PR median | delta | | --- | ---: | ---: | ---: | | `RadixUIAdoptionSection.jsx` | 41.82 us | 39.69 us | -5.09% | | `errors.ts` | 60.51 us | 56.20 us | -7.12% | | `Search.tsx` | 249.52 us | 239.56 us | -3.99% | | `core.js` | 230.19 us | 221.52 us | -3.77% | | `next.ts` | 360.07 us | 348.98 us | -3.08% | | `index.tsx` | 712.25 us | 673.43 us | -5.45% | | `handle-comments.js` | 394.35 us | 376.84 us | -4.44% | | `types.ts` | 1.393 ms | 1.304 ms | -6.42% | | `App.tsx` | 8.928 ms | 8.156 ms | -8.64% | Geomean across these formatter fixtures: -5.35% median runtime. Treat as directional local data; CI/CodSpeed is the source of truth.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The formatter's fitting loop calls
FitsMeasurer::fits_elementfor every visited format element. LLVM does not inline this dispatcher heuristically because of its size, so each visit currently pays for an out-of-line function call.This change marks
fits_elementas#[inline(always)], keeping the dispatcher in the hot fitting loop. On the non-microbenchmarkformatter[large/dataset.py], CodSpeed CPU simulation improves from 8.42 ms to 7.98 ms, a 5.5% speedup (comparison). The other measured formatter workloads improve by 1.0-4.5%, with no formatter output or memory changes.