Skip to content

ci(benchmarks): benchmark NAPI parser#9519

Merged
graphite-app[bot] merged 1 commit intomainfrom
03-03-ci_benchmarks_benchmark_napi_parser
Mar 5, 2025
Merged

ci(benchmarks): benchmark NAPI parser#9519
graphite-app[bot] merged 1 commit intomainfrom
03-03-ci_benchmarks_benchmark_napi_parser

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Mar 3, 2025

Add benchmarks for NAPI parser. Benchmarks cover both "standard" mode and "raw transfer" mode.

Codspeed say they've sorted out the problems with wide variance in NodeJS benchmarks, so hopefully they won't wave all over the place, the way they did last year when we first tried to add these benchmarks.

What we also found last year is that Codspeed's measures didn't reflect real-world wallclock measurements at all when comparing code running on Rust side vs JS side. Raw transfer moves a lot of work from Rust side to JS side, and so likely the bench results will be misleading if comparing "standard" vs "raw".

I've added benchmarks for both, not to compare the 2, but because I think there's room to improve the performance of both "standard" and "raw" independently.

Copy link
Member Author

overlookmotel commented Mar 3, 2025

@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 413d664 to aad6cb0 Compare March 3, 2025 16:30
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 3, 2025

CodSpeed Performance Report

Merging #9519 will not alter performance

Comparing 03-03-ci_benchmarks_benchmark_napi_parser (a9889f5) with main (0a5c73b)

Summary

✅ 33 untouched benchmarks
🆕 6 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 parser_napi[RadixUIAdoptionSection.jsx] N/A 4.7 ms N/A
🆕 parser_napi[pdf.mjs] N/A 1.8 s N/A
🆕 parser_napi_raw[RadixUIAdoptionSection.jsx] N/A 1.3 ms N/A
🆕 parser_napi_raw[cal.com.tsx] N/A 699.4 ms N/A
🆕 parser_napi_raw[checker.ts] N/A 1.1 s N/A
🆕 parser_napi_raw[pdf.mjs] N/A 266.2 ms N/A

@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch 2 times, most recently from a8fbf59 to 6e8e682 Compare March 3, 2025 17:13
@overlookmotel overlookmotel changed the base branch from 03-03-feat_ast_estree_raw_transfer_experimental_ to graphite-base/9519 March 3, 2025 19:42
@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from fb77f37 to d991355 Compare March 3, 2025 20:38
@overlookmotel overlookmotel changed the base branch from graphite-base/9519 to 03-03-test_ast_estree_speed_up_raw_transfer_tests March 3, 2025 20:38
@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from d991355 to 55dda90 Compare March 3, 2025 22:27
@overlookmotel overlookmotel force-pushed the 03-03-test_ast_estree_speed_up_raw_transfer_tests branch from ad7e389 to 4bd7f8c Compare March 3, 2025 22:27
@overlookmotel overlookmotel changed the base branch from 03-03-test_ast_estree_speed_up_raw_transfer_tests to graphite-base/9519 March 3, 2025 23:05
@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 55dda90 to 53f5e0d Compare March 3, 2025 23:05
@overlookmotel overlookmotel changed the base branch from graphite-base/9519 to 03-03-fix_ast_estree_raw_transfer_support_showsemanticerrors_option March 3, 2025 23:05
@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 53f5e0d to 6ea3173 Compare March 3, 2025 23:13
@overlookmotel
Copy link
Member Author

These benchmarks are really slow. I've skipped the worst ones, and sharded all the rest. But still should use a filter to skip running them unless relevant files have been touched. I'll look into that.

@overlookmotel overlookmotel force-pushed the 03-03-fix_ast_estree_raw_transfer_support_showsemanticerrors_option branch from 77d1b86 to 39b9207 Compare March 4, 2025 12:30
@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 6ea3173 to 3a97b31 Compare March 4, 2025 12:30
@overlookmotel
Copy link
Member Author

overlookmotel commented Mar 4, 2025

Argh. We can't skip these benchmarks conditionally as it'll foul up CodSpeed. Every PR that doesn't touch NAPI parser would get a "dropped benchmarks" warning.

I've reached out to Codspeed to ask if possible to allow benchmarks to be intentionally skipped, and they've put it on their issue tracker. But who knows how long it'll take.

We may need to take matters into our own hands and build something to skip benchmarks when their dependences haven't changed, and submit the previous benchmark run's results in its place to keep Codspeed happy (oxc-project/backlog#3).

But in the meantime, I think we do need benchmarks for NAPI parser, as I'm going to be working on optimizing it. I've skipped the slowest ones, so they all complete faster than the linter benchmarks anyway.

@overlookmotel overlookmotel force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 8892c2f to bcc432d Compare March 4, 2025 12:37
@Boshen Boshen changed the base branch from 03-03-fix_ast_estree_raw_transfer_support_showsemanticerrors_option to graphite-base/9519 March 4, 2025 14:42
@Boshen Boshen force-pushed the graphite-base/9519 branch from 39b9207 to a0f6f37 Compare March 4, 2025 14:53
@Boshen Boshen force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from bcc432d to 8e287f6 Compare March 4, 2025 14:53
@Boshen Boshen changed the base branch from graphite-base/9519 to main March 4, 2025 14:54
@Boshen Boshen force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 8e287f6 to 0c6bb8c Compare March 4, 2025 14:54
@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 5, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 5, 2025

Merge activity

Add benchmarks for NAPI parser. Benchmarks cover both "standard" mode and "raw transfer" mode.

Codspeed say they've sorted out the problems with wide variance in NodeJS benchmarks, so hopefully they won't wave all over the place, the way they did last year when we first tried to add these benchmarks.

What we also found last year is that Codspeed's measures didn't reflect real-world wallclock measurements at all when comparing code running on Rust side vs JS side. Raw transfer moves a lot of work from Rust side to JS side, and so likely the bench results will be misleading if comparing "standard" vs "raw".

I've added benchmarks for both, not to compare the 2, but because I think there's room to improve the performance of both "standard" and "raw" independently.
@graphite-app graphite-app bot force-pushed the 03-03-ci_benchmarks_benchmark_napi_parser branch from 49a4100 to a9889f5 Compare March 5, 2025 02:21
@graphite-app graphite-app bot merged commit a9889f5 into main Mar 5, 2025
38 checks passed
@graphite-app graphite-app bot deleted the 03-03-ci_benchmarks_benchmark_napi_parser branch March 5, 2025 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants