benchmark: add benchmark utility for Error instance by mertcanaltin · Pull Request #61180 · nodejs/node

mertcanaltin · 2025-12-27T07:17:24Z

added benchmark code for isNativeError function

util/is-native-error.js n=1000000 version="native" argument="true": 281,584,407.8828992

nodejs-github-bot · 2025-12-27T07:17:29Z

Review requested:

@nodejs/performance

RafaelGSS

I'm not confident about how useful this benchmark is

RafaelGSS · 2025-12-27T15:13:00Z

benchmark/util/is-native-error.js

+
+  bench.start();
+  for (let iteration = 0; iteration < n; iteration++) {
+    func(arg);


This is likely being optimized.

Hi, I tried to fix this by using more arguments and variable arguments. 5fcf423

anonrig · 2025-12-27T16:54:58Z

I'm not confident about how useful this benchmark is

Any recommendations?

lpinca · 2025-12-27T17:52:51Z

benchmark/util/is-native-error.js

+
+  const func = { native: util, js: types }[version].isNativeError;
+
+  const testArgs = [args[argument], args[argument], args[argument]];


Any reason for this? If I'm reading correctly, the elements are all equals.

I wanted to provide more arguments.

With

const testArg = testArgs[iteration % 3]; func(testArg)

below, I'm still not sure I understand.

You're right, it could be misleading for them all to be the same I changed them with different arguments.
9de3f6f

now "%3" alternative I use testArgs.length

After the latest changes, I got a test failure because the benchmark was running with three configurations instead of the expected one or two. I’ve updated the code so each run uses only a single argument, matching the test infrastructure’s requirements. FYİ @RafaelGSS

…mark

benchmark/util/is-native-error.js

RafaelGSS · 2025-12-30T12:26:34Z

benchmark/util/is-native-error.js

+  const util = common.binding('util');
+  const types = require('internal/util/types');
+
+  const func = { native: util, js: types }[version].isNativeError;


Isn't recently deprecated?

node/doc/api/deprecations.md

Line 4283 in 20bf328

### DEP0197: `util.types.isNativeError()`

it is deprecated but still supported - we should still not regress on stable releases and this change ensures that @RafaelGSS

But, what's the point of this benchmark? I mean, if we see util/isNativeError is slower, what it means afterall? Which API will be directly impacted besides the deprecated API?

This comparison ensures that existing users will not experience performance drops or unexpected slowdowns until the API is completely removed. It also aims to provide a reference for future changes or alternatives.

benchmark/util/is-native-error.js

RafaelGSS

I'm -1 on this for the reasons I mentioned in #61180 (comment).

We are trying to make the whole benchmark suite faster with more meaningful results and it's not clear to me what outcome we expect from this benchmark - keep in mind this is a deprecated API.

lemire · 2026-01-04T18:55:34Z

@RafaelGSS

Isn't the answer 'modularity' ? E.g., have a set of core benchmarks that are tracked... E.g., core functionality that one does not want to get slower... And then other sets of benchmarks that are selectively run.

RafaelGSS · 2026-01-04T18:58:09Z

@RafaelGSS

Isn't the answer 'modularity' ? E.g., have a set of core benchmarks that are tracked... E.g., core functionality that one does not want to get slower... And then other sets of benchmarks that are selectively run.

@lemire That would only work if we had sufficient patterns to identify which 'module' to run based on each change. Currently, we don't have any mechanism that identifies: if someone changes the file/function/column X, which benchmark suite I would need to run to guarantee no regression. We can only run the full set.

mertcanaltin · 2026-01-07T22:12:08Z

Thank you all for your comments. @lemire, would you like to comment here? #61180 (comment)

mertcanaltin · 2026-01-11T14:25:09Z

@RafaelGSS I understand your concern about adding benchmarks for deprecated APIs. However, this benchmark isn't solely about isNativeError itself.
As mentioned in the PR description, this is related to #60468 The logger API will need to perform error detection internally, and having a baseline benchmark for error instance checking

helps us:
Ensure any future error detection mechanism (whether we use isNativeError, instanceof Error, or another approach) doesn't introduce performance regressions
Compare different implementation strategies for error handling in the logger API

If you still think this doesn't justify the benchmark, I'm open to closing this PR and revisiting it when the logger API implementation is more concrete. What do you think?

RafaelGSS · 2026-01-11T21:57:40Z

Ensure any future error detection mechanism (whether we use isNativeError, instanceof Error, or another approach) doesn't introduce performance regressions

Why would we use the isNativeError if that's deprecated?

As mentioned in the PR description, this is related to #60468 The logger API will need to perform error detection internally, and having a baseline benchmark for error instance checking

If that's necessary for #60469, I suggest including this commit in that PR instead - and make it meaningful.

mertcanaltin · 2026-01-21T17:46:08Z

Ensure any future error detection mechanism (whether we use isNativeError, instanceof Error, or another approach) doesn't introduce performance regressions

Why would we use the isNativeError if that's deprecated?

As mentioned in the PR description, this is related to #60468 The logger API will need to perform error detection internally, and having a baseline benchmark for error instance checking

If that's necessary for #60469, I suggest including this commit in that PR instead - and make it meaningful.

Thanks for your comments, @RafaelGSS I'll move this to the PR where the logger API is and make it meaningful by comparing different error detection approaches.

benchmark: add benchmark utility for isNativeError function

3b40faf

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. util Issues and PRs related to the built-in util module. labels Dec 27, 2025

mertcanaltin mentioned this pull request Dec 27, 2025

[WIP] lib: added logger api in node core #60468

Open

RafaelGSS reviewed Dec 27, 2025

View reviewed changes

repair benchmark

5fcf423

anonrig approved these changes Dec 27, 2025

View reviewed changes

lpinca reviewed Dec 27, 2025

View reviewed changes

mertcanaltin added 2 commits December 28, 2025 10:55

refine test arguments and measure elapsed time in isNativeError bench…

9de3f6f

…mark

simplify isNativeError test arguments and remove performance logging

c4aaa5d

RafaelGSS reviewed Dec 30, 2025

View reviewed changes

mertcanaltin added 2 commits December 30, 2025 19:43

store result of isNativeError function for validation

23955ed

add benchmark utility for isError function

f2e53f1

mertcanaltin changed the title ~~benchmark: add benchmark utility for isNativeError function~~ benchmark: add benchmark utility for Error instance Dec 30, 2025

mertcanaltin added 2 commits December 30, 2025 20:05

refactor benchmark utility for isError function to improve readability

6b26012

add benchmark utility for isNativeError function

8c79e29

H4ad reviewed Jan 3, 2026

View reviewed changes

benchmark/util/is-native-error.js Outdated Show resolved Hide resolved

Merge branch 'main' into mert/create/benchmark/isNativeError

a0f0062

RafaelGSS requested changes Jan 4, 2026

View reviewed changes

mertcanaltin requested review from H4ad, RafaelGSS, anonrig and lpinca January 11, 2026 14:26

H4ad approved these changes Jan 11, 2026

View reviewed changes

mertcanaltin closed this Jan 21, 2026


		const func = { native: util, js: types }[version].isNativeError;

		const testArgs = [args[argument], args[argument], args[argument]];

Uh oh!

Conversation

mertcanaltin commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Dec 27, 2025

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Dec 27, 2025 • 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!

anonrig commented Dec 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Dec 28, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RafaelGSS Dec 30, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

lemire commented Jan 4, 2026

Uh oh!

RafaelGSS commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mertcanaltin commented Jan 7, 2026

Uh oh!

mertcanaltin commented Jan 11, 2026

Uh oh!

RafaelGSS commented Jan 11, 2026

Uh oh!

mertcanaltin commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mertcanaltin commented Dec 27, 2025 •

edited

Loading

mertcanaltin Dec 27, 2025 •

edited

Loading

mertcanaltin Dec 28, 2025 •

edited

Loading

RafaelGSS Dec 30, 2025 •

edited

Loading

RafaelGSS commented Jan 4, 2026 •

edited

Loading

mertcanaltin commented Jan 21, 2026 •

edited

Loading