Skip to content

Comments

perf(cfg, codegen, linter, syntax): use NonMaxU32 from oxc_data_structures#9886

Closed
overlookmotel wants to merge 1 commit into03-19-feat_data_structures_zero-cost_nonmaxu32_typefrom
03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_
Closed

perf(cfg, codegen, linter, syntax): use NonMaxU32 from oxc_data_structures#9886
overlookmotel wants to merge 1 commit into03-19-feat_data_structures_zero-cost_nonmaxu32_typefrom
03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Mar 19, 2025 •
edited
Loading

Replace nonmax dependency with our own NonMaxU32 type, introduced in #9883.

@github-actions github-actions bot added A-linter Area - Linter A-codegen Area - Code Generation A-cfg Area - Control Flow Graph A-ast-tools Area - AST tools C-performance Category - Solution not expected to change functional behavior, only performance labels Mar 19, 2025
This was referenced Mar 19, 2025
Closed
Merged
Merged
@overlookmotel Graphite App
Copy link
Member Author

overlookmotel commented Mar 19, 2025 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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
Copy link

codspeed-hq bot commented Mar 19, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #9886 will create unknown performance changes

Comparing 03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_ (f6c0e25) with 03-19-feat_data_structures_zero-cost_nonmaxu32_type (4d4a502)

Summary

🆕 33 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 codegen[checker.ts] N/A 22.6 ms N/A
🆕 codegen_sourcemap[checker.ts] N/A 65.7 ms N/A
🆕 isolated-declarations[vue-id.ts] N/A 57.8 ms N/A
🆕 lexer[RadixUIAdoptionSection.jsx] N/A 20.9 µs N/A
🆕 lexer[antd.js] N/A 24.1 ms N/A
🆕 lexer[cal.com.tsx] N/A 5.7 ms N/A
🆕 lexer[checker.ts] N/A 14.5 ms N/A
🆕 lexer[pdf.mjs] N/A 3.8 ms N/A
🆕 linter[RadixUIAdoptionSection.jsx] N/A 2.7 ms N/A
🆕 linter[cal.com.tsx] N/A 1.2 s N/A
🆕 linter[checker.ts] N/A 2.9 s N/A
🆕 mangler[antd.js] N/A 15.7 ms N/A
🆕 mangler[react.development.js] N/A 291.5 µs N/A
🆕 mangler[typescript.js] N/A 39.1 ms N/A
🆕 minifier[antd.js] N/A 163.2 ms N/A
🆕 minifier[react.development.js] N/A 1.8 ms N/A
🆕 minifier[typescript.js] N/A 288.3 ms N/A
🆕 estree[checker.ts] N/A 95.4 ms N/A
🆕 parser[RadixUIAdoptionSection.jsx] N/A 90.9 µs N/A
🆕 parser[antd.js] N/A 112.4 ms N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@overlookmotel overlookmotel changed the base branch from 03-19-feat_data_structures_zero-cost_nonmaxu32_type to graphite-base/9886 March 19, 2025 08:04
@overlookmotel overlookmotel mentioned this pull request Mar 19, 2025
Merged
@overlookmotel overlookmotel force-pushed the 03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_ branch from fccfa3f to 5158b40 Compare March 19, 2025 08:17
@overlookmotel overlookmotel changed the base branch from graphite-base/9886 to 03-19-feat_data_structures_zero-cost_nonmaxu32_type March 19, 2025 08:17
@graphite-app graphite-app bot force-pushed the 03-19-feat_data_structures_zero-cost_nonmaxu32_type branch from 9694667 to a111895 Compare March 19, 2025 09:04
@graphite-app graphite-app bot force-pushed the 03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_ branch from 5158b40 to e3c7249 Compare March 19, 2025 09:04
@overlookmotel
Copy link
Member Author

overlookmotel commented Mar 19, 2025

I'm not sure if this is worthwhile or not. It's showing a small speedup on semantic benchmarks, and +1% on mangler.

It looks like the biggest benefit is in sorting slices of NonZeroU32s (e.g. SymbolIds). But there may be a simpler way to get that benefit - nonmax has quite an odd Ord implementation, which could maybe be improved.

Godbolt shows fast path for indexing into an IndexVec with the new NonZeroU32 is actually 1 more instruction than it was before. https://godbolt.org/z/x7KPr3Wxj (though it can be reduced with addition of assert!(index < slice.len());).

@graphite-app graphite-app bot force-pushed the 03-19-feat_data_structures_zero-cost_nonmaxu32_type branch 2 times, most recently from 40ea7e0 to 4d4a502 Compare March 20, 2025 01:09
@graphite-app graphite-app bot force-pushed the 03-19-perf_cfg_codegen_linter_syntax_use_nonmaxu32_from_oxc_data_structures_ branch from e3c7249 to f6c0e25 Compare March 20, 2025 01:09
@overlookmotel overlookmotel self-assigned this Sep 10, 2025
@Boshen Boshen closed this Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

@overlookmotel overlookmotel

Labels

A-ast-tools Area - AST tools A-cfg Area - Control Flow Graph A-codegen Area - Code Generation A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@overlookmotel @Boshen