benchmark: add test runner hooks and options by luanmuniz · Pull Request #63754 · nodejs/node

luanmuniz · 2026-06-05T15:21:18Z

Add benchmarks for node:test hooks and test options. The hooks benchmark covers before, after, beforeEach, and afterEach, with a none mode as the baseline. The test options benchmark covers skip and todo behavior. This adds coverage for part of the benchmark/test_runner gaps tracked in the issue.

Refs: #55723

I eventually want to continue working on the Ref issue, and do the other benchmarks, but I decided to start with the simple ones that I can use existing files as a reference, get a feel for how the project works. Starting small and all of that. This will be my first (hopefully of many) contribution.

Please let me know if any changes or actions are required from me. I will wait for feedback before start tackling the other parts of the ref issue

Edit: I had some time today, and I started working on the other benchmarks, and I do have only, mock.timers and mock.module benchmarks ready too. I will await for feedback on the current code before pushing the other benchmarks. Please let me know if you prefer for me to push a different PR those since they have a bit more nuance, especially the mock.module one (Still nothing major tho)

nodejs-github-bot · 2026-06-05T15:21:24Z

Review requested:

@nodejs/performance

luanmuniz · 2026-06-07T19:35:05Z

@avivkeller Changes done!

Obs: suite-tests.js, global-sequential-tests.js and global-concurrent-tests.js Seem to have the same switch problem. Should I fix that and include it in this PR?

RafaelGSS · 2026-06-07T21:41:51Z

+ // eslint-disable-next-line prefer-const
+ let avoidV8Optimization = 0;


why 0 and why let?

It is let because we rewrite the values through the benchmark, and there is no particular reason to be 0, just didn't want to leave it undefined, and the original value of the variable shouldn't have an impact on this.
To be 100% honest, I don't quite understand the mechanics for this. I never dug into v8 optimizations before. The variable is there because I decided to follow the existing patterns inside this particular folder.
But please check this reference link and this for the comment that originally introduced this specific variable (Seems like the link is not working 100%, opening the discussion so just crtl+f for benchmark/test_runner/global-concurrent-tests.js and there should be a resolved review from H4ad with the reference)
Obs: I just realized the hooks are noops, and given that this variable works as described, I shouldn't have a noop there, so I'm pushing code so the hooks also have minimal execution within the function. Thanks for calling my attention to this.

luanmuniz · 2026-06-10T18:30:32Z

@RafaelGSS @avivkeller Both feedbacks are addressed. Please let me know if it's resolved or if any changes are still needed.

luanmuniz · 2026-06-17T09:06:28Z

Ping @RafaelGSS @avivkeller

I'm waiting for this PR to be done so I can continue working on the other benchmarks and hopefully close the Ref issue.
Please let me know of any requirements here. Thank you!

I have additional local benchmarks for mock.timers, mock.module, and only, but I’ll keep this PR scoped to hooks/options unless maintainers prefer that I expand it. Happy to split follow-ups however is easiest to review.

RafaelGSS · 2026-06-23T20:41:05Z

+} = require('node:test');
+
+const bench = common.createBenchmark(main, {
+ n: [1, 10, 100, 1000],


Are these numbers arbitrarily chosen? Please use https://github.com/nodejs/node/blob/main/benchmark/calibrate-n.js

Yes and No. I took those numbers from another benchmark in the same folder since I didn't know exactly how the configurations were used forward.
That being said, I didn't know about calibrate-n and how it worked, but after some study, I think I understand it now. I'm running it for both this and the PR for mock-timers. I will post the results here as soon as I have them and update the numbers with the best one found.

RafaelGSS

I'm uncertain that the number of configs for the benchmarks is useful. Our benchmark suite is already too big, any configuration you add increases the suite run exponentially.

It would be great to share the current results too.

RafaelGSS · 2026-06-23T20:42:33Z

+const { it } = require('node:test');
+
+const bench = common.createBenchmark(main, {
+ n: [1, 10, 100, 1000],


please use calibrate N here.

luanmuniz · 2026-06-25T11:10:39Z

Hi there, sorry for the force-push. I had a local git history issue while cleaning up the branch, and I force-pushed to get the PR back into the intended state. Apologies for the noise. I’ll be more careful with the branch history going forward.

I've used calibrate-n, and there are the results in case you want to take a look:
calibrate-hooks.txt
calibrate-test-options.txt

The script asked for 10K for both benchmarks, but in my opinion, the hooks.js was taking too long to run with 10K, so I went with the second-best choice there.

Also, I found a small bug in the calibrate-n code: it wasn't using the correct arguments and was always using the const defaults instead. I just opened a PR for it (#64146)

Edit: I just saw some commit messages have more than 72 chars per line. I will fix this now and force-push-with-lease in a few minutes to fix the issue so CI doesn't break

CC @RafaelGSS

Add benchmarks for node:test hooks and test options. The hooks benchmark covers before, after, beforeEach, and afterEach, with a none mode as the baseline. The test options benchmark covers skip and todo behavior. This adds coverage for part of the benchmark/test_runner gaps tracked in the issue. Refs: nodejs#55723 Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

Select each benchmark variant before starting the timer so the measured loop does not include dispatch overhead. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

Update the hooks benchmark so registered hooks perform the same anti-optimization assignment as test bodies instead of calling a noop. This keeps the measured hook path from using an empty callback. Links for more information on this actions: nodejs#48931 (comment)https://www.mail-archive.com/v8-users@googlegroups.com/msg05521.html Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

Use calibrated n values for the test runner hooks and options benchmarks. Reduce the test options benchmark configurations to keep the benchmark suite smaller. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

luanmuniz · 2026-07-03T09:34:00Z

Ping @RafaelGSS @avivkeller
Friendly ping on this one.

I believe the review feedback has been addressed, but please let me know if there is anything else you’d like me to change before this can move forward.

Happy to adjust further if needed.

avivkeller

Sorry for the delay!

luanmuniz · 2026-07-10T08:50:11Z

@RafaelGSS Sorry to keep pinging, im waiting the existing PR's to be closed to continue working on the other benchmarks, i don't want to have more PR's open at the same time. Let me know if there is anything i can do to help you move this and the other tickets i have open. Thank you!

luanmuniz · 2026-07-10T19:52:53Z

Seems like the macOS CI failed in parallel/test-debugger-break. This appears to be unrelated to this PR, which only adds files under benchmark/test_runner.

Could someone please rerun the failed job? I don’t have permission to rerun.

luanmuniz · 2026-07-17T13:00:29Z

@nodejs/benchmarking Friendly follow-up on the CI failure here.

This has an approval from @avivkeller after the latest changes and has been open for more than seven days. The only failing GitHub Actions job is the unrelated macOS test-debugger-break timeout; the Linux jobs passed.

Could someone please rerun the failed CI and, if it passes, add this to the commit queue?

luanmuniz · 2026-07-22T18:01:36Z

Thanks! CI was retriggered and is now passing on the current HEAD.
Since this PR already has approval and is marked author ready, could someone please add it to the commit queue?

RafaelGSS · 2026-07-23T12:25:55Z

+ for (let i = 0; i < loopAmount; i++) {
+ describe(`${i}`, () => {
+ hookFn(() => {
+ avoidV8Optimization = i;


That's still unnecessary. This is not what will prevent V8 from optimising this code away. This variable name just provide a false sense this is the reason this code won't be optimized away.

@RafaelGSS Can you provide more details for your suggestion? Is your suggestion removing the variable and leaving it as a noop, or renaming it? If renaming it, could you suggest something? That way we don't delay this PR further with naming discussion.

removing it and leaving as noop

Done! Removed avoidV8Optimization and left the callbacks as no-ops. If the current implementation is acceptable, could the thread be resolved and CI started?

Due to lack of evidence and review requirement the avoidV8Optimization variable was removed and left the callbacks as no-ops. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. test_runner Issues and PRs related to the test runner subsystem. labels Jun 5, 2026

avivkeller reviewed Jun 7, 2026
View reviewed changes

Comment threadbenchmark/test_runner/hooks.js Outdated

RafaelGSS reviewed Jun 7, 2026
View reviewed changes

luanmuniz requested review from RafaelGSS and avivkeller June 9, 2026 11:06

luanmuniz mentioned this pull request Jun 23, 2026
benchmark: add test-only and mock timers cases #64097
Open

RafaelGSS reviewed Jun 23, 2026
View reviewed changes

luanmuniz force-pushed the benchmark-test-runner-hooks-options branch from d8f7882 to d4ab1a9Compare June 25, 2026 10:54

luanmuniz requested a review from RafaelGSS June 25, 2026 11:58

luanmuniz added 4 commits June 27, 2026 00:10

benchmark: avoid switch in test runner benchmarks
b9188c0
Select each benchmark variant before starting the timer so the measured loop does not include dispatch overhead. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

benchmark: calibrate test runner benchmarks
d09c413
Use calibrated n values for the test runner hooks and options benchmarks. Reduce the test options benchmark configurations to keep the benchmark suite smaller. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

luanmuniz force-pushed the benchmark-test-runner-hooks-options branch from d4ab1a9 to d09c413Compare June 26, 2026 22:11

avivkeller approved these changes Jul 3, 2026
View reviewed changes

avivkeller added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 10, 2026

RafaelGSS reviewed Jul 23, 2026
View reviewed changes

benchmark: remove optimization guard
c39085e
Due to lack of evidence and review requirement the avoidV8Optimization variable was removed and left the callbacks as no-ops. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>

RafaelGSS approved these changes Jul 27, 2026
View reviewed changes

		// eslint-disable-next-line prefer-const
		let avoidV8Optimization = 0;

Uh oh!

Uh oh!

Conversation

luanmuniz commented Jun 5, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 5, 2026

Uh oh!

Uh oh!

luanmuniz commented Jun 7, 2026

Uh oh!

RafaelGSSJun 7, 2026

Choose a reason for hiding this comment

Uh oh!

luanmunizJun 8, 2026

Choose a reason for hiding this comment

Uh oh!

luanmuniz commented Jun 10, 2026

Uh oh!

luanmuniz commented Jun 17, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RafaelGSSJun 23, 2026

Choose a reason for hiding this comment

Uh oh!

luanmunizJun 24, 2026

Choose a reason for hiding this comment

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

RafaelGSSJun 23, 2026

Choose a reason for hiding this comment

Uh oh!

luanmuniz commented Jun 25, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luanmuniz commented Jul 3, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avivkeller left a comment

Choose a reason for hiding this comment

Uh oh!

luanmuniz commented Jul 10, 2026

Uh oh!

luanmuniz commented Jul 10, 2026

Uh oh!

luanmuniz commented Jul 17, 2026

Uh oh!

luanmuniz commented Jul 22, 2026

Uh oh!

RafaelGSSJul 23, 2026

Choose a reason for hiding this comment

Uh oh!

luanmunizJul 23, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RafaelGSSJul 25, 2026

Choose a reason for hiding this comment

Uh oh!

luanmunizJul 26, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

luanmuniz commented Jun 5, 2026•
edited
Loading

luanmuniz commented Jun 17, 2026•
edited
Loading

luanmuniz commented Jun 25, 2026•
edited
Loading

luanmuniz commented Jul 3, 2026•
edited
Loading

luanmunizJul 23, 2026•
edited
Loading

luanmunizJul 26, 2026•
edited
Loading