Skip to content
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

Benchmark NodeJS parser #2616

Closed
overlookmotel opened this issue Mar 5, 2024 · 2 comments · Fixed by #2724
Closed

Benchmark NodeJS parser #2616

overlookmotel opened this issue Mar 5, 2024 · 2 comments · Fixed by #2724

Comments

@overlookmotel
Copy link
Contributor

For #2457 and also the work around ESTree-izing the parser's JSON output, it'd be good to be benchmarking the NodeJS oxc-parser interface.

Any idea how we could go about this? CodSpeed says it supports NodeJS. But:

Should we create a separate set of benchmarks in CodSpeed for it, or try to get it added to the current list?

If NodeJS OXC parser is a primary performance target, it could be good to add it to main list, so we see any performance regressions/improvements on every commit, same as we do for the Rust interfaces.

A hacky way to do this would be:

  • Add a benchmark implemented in Rust.
  • That benchmark starts up NodeJS as a child process.
  • On each turn of the benchmark loop in Rust, write a line to child process's stdin.
  • NodeJS process waits to receive a line on stdin, runs parser, and then writes a line to stdout to signal that it's done.
  • Rust benchmark code waits until it receives a line on child process's stdout, then completes the turn of the benchmark loop.

Purpose of co-ordinating over stdin/stdout is avoid creating and destroying a NodeJS child process on each turn of the benchmark loop, as that overhead would be high, and not representative of real-world usage where typically you'd parse many files in a single NodeJS process.

But is there a better way than this?

@Boshen
Copy link
Member

Boshen commented Mar 6, 2024

It seems like all we need to bench is the parse_sync_raw function. Is it ok to assume the actual memory transfer is a constant?

Benchmarking is node.js is notoriously unstable, I don't want to spend time on the node.js side :-)

@overlookmotel
Copy link
Contributor Author

overlookmotel commented Mar 6, 2024

Somewhat. JSON.parse on JS side is out of our control, so we can't optimize it. So not much point measuring it.

The overhead of NAPI-RS would be good to measure though. I wonder if, for instance, passing the JSON from Rust to JS as a buffer instead of a string would be faster.

But mainly I'm thinking of #2457, which involves a lot of JS-side code for deserialization. To measure how effective that is, we would ideally benchmark the whole thing end-to-end (inc both JS and Rust).

Understood you'd prefer to concentrate your efforts on Rust side. I get that! JS is messy. I'll look into it when I have time. I opened this issue mainly in case you might say "I've done this before. We should do X".

Boshen pushed a commit that referenced this issue Mar 15, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants