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

Exposing NodeJS Benchmarking functionality as a module #45037

Closed
zombieleet opened this issue Oct 17, 2022 · 6 comments
Closed

Exposing NodeJS Benchmarking functionality as a module #45037

zombieleet opened this issue Oct 17, 2022 · 6 comments
Labels
benchmark Issues and PRs related to the benchmark subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@zombieleet
Copy link
Contributor

zombieleet commented Oct 17, 2022

What is the problem this feature will solve?

This feature introduces a new node:benchmark module (it exposes the internal benchmarking functionality that already exists in node core bencharmk, which serves the purpose of measuring the time taken for an operation to complete.

The module will also support benchmarking HTTP(s) requests using either autocannon, h2load or wrk http-bencharmks.

The module will also support and provide a user/developer friendly interface to compare benchmark results across two nodejs versions. compare-benchmark

What is the feature you are proposing to solve the problem?

The node:benchmark module gives a more reliable approach towards benchmarking a set of operations (The existing solution in node core might be also improved on to provide more functionality).

There is currently a draft-pr on this (with few reviews on this).

What alternatives have you considered?

No response

@zombieleet zombieleet added the feature request Issues that request new features to be added to Node.js. label Oct 17, 2022
@tniessen
Copy link
Member

The standard questions when adding new public APIs include:

  1. Is inclusion in core desired by the community?
  2. Is it difficult or impossible to properly implement this outside of node core?

The node:benchmark module gives a more reliable approach towards benchmarking a set of operations

More reliable than what?


The internal benchmarking tools were mostly designed to compare the performance of a modified Node.js build against that of the unmodified build. I am not sure if this is what users would be looking for. (Then again, I don't recall much demand for built-in benchmarking tools.)

@tniessen tniessen added the benchmark Issues and PRs related to the benchmark subsystem. label Oct 17, 2022
@zombieleet
Copy link
Contributor Author

@tniessen

  1. It's not something I see most people desiring to exist in node core, probably because there is a widespread use of more naive approaches like using Date.now() and/or peformance.now().
  2. It's not difficult neither is it impossible, but having it in core is a more reasonable approach in terms of security giving the constant rise of supply chain attacks.

Also, the implementation in core might have been designed for node specific usage, but it can be used outside node core. In terms of been developer/user friendly, I have done a great deal of work (subject to review) on this.

They might not be a high demand for a benchmarking solution as a module as of jow, but having one that gives a more precise result rather than using the naive approaches will be favourable.

@tniessen
Copy link
Member

It's not difficult neither is it impossible, but having it in core is a more reasonable approach in terms of security giving the constant rise of supply chain attacks.

This can be said about every single npm module, and yet we won't include every single npm module in node core :)

gives a more precise result rather than using the naive approaches

Is this a fact? Is our internal benchmarking tool more precise than just using process.hrtime.bigint() and a sensible benchmarking setup?

@zombieleet
Copy link
Contributor Author

zombieleet commented Oct 18, 2022

@tniessen we can also change process.hrtime() in the node implementation to process.hrtime.bigint() and effect any necessary change (I am open to doing this as well), the benchmarking setup in node core is sensible as far as I know :).

@joyeecheung
Copy link
Member

joyeecheung commented Oct 18, 2022

The internal benchmarking tools were mostly designed to compare the performance of a modified Node.js build against that of the unmodified build. I am not sure if this is what users would be looking for. (Then again, I don't recall much demand for built-in benchmarking tools.)

I agree with this, I am not sure how much the users can benefit from the current tools we have. They are not designed to compare the performance of different pieces of user code run by the same binary, instead they are designed to compare the performance of different Node.js binaries running the same piece of user code. But in the wild I think the most users are only interested in the former. It can be possible to use the tools for the first use case in that you can write different benchmarks with different ways of implementing a user-land thing, and then you check the numbers coming out of run.js, but compare.js is not designed for comparing across different benchmarks, and using run.js for that use case isn't much better than doing process.hrtime() yourselves. On the other hand the benchmarking tool we have are mostly used to do microbenchmarks which is difficult to get right (people write code in a loop that gets folded by V8 and run into false "regressions" all the time). For users I'd say a benchmarking tool for testing more realistic use cases would be better, maybe the http part of the benchmarking tool would be useful, but there are already many better runtime-independent solutions, and they tend to be more tailored to the platform that you are testing on as well, so having a less battle-tested one in Node.js core doesn't provide much additional benefit either..

@zombieleet
Copy link
Contributor Author

Okay, I agree with what has been said so far. I will close the Issue and PR.

Thanks for taking the time to look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

3 participants