perf(napi/parser): faster fixup of BigInts and RegExps#10820
Merged
graphite-app[bot] merged 1 commit intomainfrom May 6, 2025
Merged
Conversation
github-actions
bot
added
the
C-performance
Member
Author
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 Instrumentation Performance ReportMerging #10820 will degrade performances by 4.89%Comparing Summary
Benchmarks breakdown
|
Member
Author
|
Regression in |
camc314
reviewed
May 5, 2025
Member
Merge activity
|
#10791 fixed the performance of `oxc-parser` NPM package by removing the reviver from `JSON.parse` call. However, it replaced it with a complete traversal of the AST on JS side to locate `Literal` nodes and update them. Instead, generate a list of paths to `Literal` nodes needing fixing on Rust side in `ESTree` serializer. e.g. for this program: ```js 123n; foo(/xyz/); ``` the fix paths are: ```json [ ["body", 0, "expression"], ["body", 1, "expression", "arguments", 2] ] ``` Having the location of nodes which need updating reduces work on JS side, as no need to search the entire AST. Running `pnpm run bench` (in `napi/parser`) locally shows between 8% and 15% speed-up from this change.
graphite-app
bot
force-pushed
the
05-05-ci_benchmarks_estree_benchmark_use_black_box_on_result_of_to_estree_ts_json_
branch
from
2362045 to
33969ac
Compare
graphite-app
bot
force-pushed
the
05-05-perf_napi_parser_faster_fixup_of_bigint_s_and_regexp_s
branch
from
514f81f to
49a6f97
Compare
Base automatically changed from
05-05-ci_benchmarks_estree_benchmark_use_black_box_on_result_of_to_estree_ts_json_
to
main
May 6, 2025 02:41
graphite-app
bot
deleted the
05-05-perf_napi_parser_faster_fixup_of_bigint_s_and_regexp_s
branch
graphite-app bot
pushed a commit
that referenced
this pull request
May 7, 2025
…on separate lines (#10869) #10820 changed how `BigInt`s and `RegExp`s are fixed in ESTree AST on JS side. It adds a list of fix paths to the end of the JSON. Add line breaks after the inserted header, and before the inserted footer. This will allow playground to easily trim off the header and footer and display the raw JSON instead of converting the AST Object back to JSON with `JSON.stringify`. oxc-project/playground#101
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
4 participants
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.
#10791 fixed the performance of
oxc-parserNPM package by removing the reviver fromJSON.parsecall. However, it replaced it with a complete traversal of the AST on JS side to locateLiteralnodes and update them.Instead, generate a list of paths to
Literalnodes needing fixing on Rust side inESTreeserializer.e.g. for this program:
the fix paths are:
[ ["body", 0, "expression"], ["body", 1, "expression", "arguments", 2] ]Having the location of nodes which need updating reduces work on JS side, as no need to search the entire AST.
Running
pnpm run bench(innapi/parser) locally shows between 8% and 15% speed-up from this change.