-
-
Notifications
You must be signed in to change notification settings - Fork 484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(napi): add oxc-parser
to benchmarks
#2724
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @overlookmotel and the rest of your teammates on Graphite |
549ec9a
to
6e0d135
Compare
NB: I initially tried making the JS benchmarks a separate task, but codspeed then only displays 1 set of benchmarks or the other - whichever results arrive latest clobber the other task's results. It seems all benchmarks have to be in a single call to their |
Had to update pnpm as was failing to install with Seems to be a common problem with pnpm and only apparent solution is to update to latest. PS: Benchmarks are getting quite slow on CI. Maybe we do need to look into trying to shard them across multiple tasks. |
CodSpeed Performance ReportMerging #2724 will not alter performanceComparing 🎉 Hooray!
|
Benchmark | main |
03-15-chore_napi_add_oxc-parser_to_benchmarks |
Change | |
---|---|---|---|---|
🆕 | parser(napi)[RadixUIAdoptionSection.jsx] |
N/A | 6.3 ms | N/A |
🆕 | parser(napi)[antd.js] |
N/A | 15.3 s | N/A |
🆕 | parser(napi)[cal.com.tsx] |
N/A | 4 s | N/A |
🆕 | parser(napi)[checker.ts] |
N/A | 6.7 s | N/A |
🆕 | parser(napi)[pdf.mjs] |
N/A | 2.5 s | N/A |
6ecffb4
to
a2ee4ee
Compare
Let me call for help. Edit: https://discord.com/channels/1065233827569598464/1218029341741420635 |
I think it's basically the same problem as sharding the benchmarks (CodSpeedHQ/action#92). If we can figure out how to do that, we can also move NodeJS/NAPI benchmark into a separate task. From looking at https://github.com/CodSpeedHQ/runner, I do think this should be possible to do, though it'll require some experiementation. I'm really keen to get the NAPI benchmarks up and running, so I can get #2457 to a state where it can be merged. Would you consider merging this now, and then returning to the sharding problem later? |
Ok let's see what happens. |
#2724 added CodSpeed benchmarks for NodeJS `oxc-parser`. Unfortunately it turns out CodSpeed's results are wildly inaccurate. Unclear why, but have raised an issue with CodSpeed (CodSpeedHQ/action#96). In meantime it seems best to remove the benchmarks as they're not useful at present.
Add NodeJS parser to benchmarks. Previous attempt #2724 did not work due CodSpeed producing very inaccurate results (CodSpeedHQ/action#96). This version runs the actual benchmarks without CodSpeed's instrumentation. Then another faux-benchmark runs within Codspeed's instrumented action and just performs meaningless calculations in a loop for as long as is required to take same amount of time as the original uninstrumented benchmarks took. It's unfortunate that we therefore don't get flame graphs on CodSpeed, but this seems to be the best we can do for now.
Closes #2616.
Adds benchmarks for NodeJS NAPI build. Measurement includes
JSON.parse
of the AST on JS side, since that's how it'll be used 99% of the time.Benchmarks run against same files as Rust parser benchmarks, so we can see the overhead of transferring AST to JS.