Skip to content

errors: improve performance of determineSpecificType - #49696

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Uzlopak:improve-determine-specific-type
Sep 30, 2023
Merged

errors: improve performance of determineSpecificType#49696
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Uzlopak:improve-determine-specific-type

Conversation

@Uzlopak

@UzlopakUzlopak commented Sep 18, 2023

Copy link
Copy Markdown
Contributor

This PR improves the function determineSpecificType in errors.js.

Also fixes a bug, where a string would be sliced without the single tick at the end of the string.

Added unit tests. Added benchmarks based on unit tests.

main:

error/determine-specific-type.js v="() => 1n" n=1000000: 6,843,920.840090735
error/determine-specific-type.js v="() => true" n=1000000: 9,456,016.191725645
error/determine-specific-type.js v="() => false" n=1000000: 8,851,588.609212818
error/determine-specific-type.js v="() => 2" n=1000000: 9,901,905.486054687
error/determine-specific-type.js v="() => +0" n=1000000: 8,973,347.793105612
error/determine-specific-type.js v="() => -0" n=1000000: 10,751,000.496169424
error/determine-specific-type.js v="() => NaN" n=1000000: 10,215,963.841045486
error/determine-specific-type.js v="() => Infinity" n=1000000: 10,320,438.148834981
error/determine-specific-type.js v="() => \"\"" n=1000000: 6,254,431.929555482
error/determine-specific-type.js v="() => Symbol(\"foo\")" n=1000000: 5,327,156.757878972
error/determine-specific-type.js v="() => function foo() {}" n=1000000: 28,370,147.240213074
error/determine-specific-type.js v="() => null" n=1000000: 84,447,833.61750029
error/determine-specific-type.js v="() => undefined" n=1000000: 143,308,256.7768683
error/determine-specific-type.js v="() => new Array()" n=1000000: 17,252,357.461324573
error/determine-specific-type.js v="() => new BigInt64Array()" n=1000000: 21,056,043.101214882
error/determine-specific-type.js v="() => new BigUint64Array()" n=1000000: 22,987,115.95138041
error/determine-specific-type.js v="() => new Int8Array()" n=1000000: 22,238,571.03126347
error/determine-specific-type.js v="() => new Int16Array()" n=1000000: 20,920,369.479666267
error/determine-specific-type.js v="() => new Int32Array()" n=1000000: 22,552,915.624017186
error/determine-specific-type.js v="() => new Float32Array()" n=1000000: 22,731,474.433672026
error/determine-specific-type.js v="() => new Float64Array()" n=1000000: 21,389,880.191003073
error/determine-specific-type.js v="() => new Uint8Array()" n=1000000: 22,209,250.046039775
error/determine-specific-type.js v="() => new Uint8ClampedArray()" n=1000000: 21,585,063.757529628
error/determine-specific-type.js v="() => new Uint16Array()" n=1000000: 22,708,905.351735055
error/determine-specific-type.js v="() => new Uint32Array()" n=1000000: 22,544,499.855568662
error/determine-specific-type.js v="() => new Date()" n=1000000: 27,342,526.528744582
error/determine-specific-type.js v="() => new Map()" n=1000000: 26,338,839.423816815
error/determine-specific-type.js v="() => new WeakMap()" n=1000000: 25,778,798.114064377
error/determine-specific-type.js v="() => new Object()" n=1000000: 20,142,102.12766866
error/determine-specific-type.js v="() => Promise.resolve(\"foo\")" n=1000000: 26,109,976.21198397
error/determine-specific-type.js v="() => new Set()" n=1000000: 25,905,614.964326676
error/determine-specific-type.js v="() => new WeakSet()" n=1000000: 25,752,138.193596177
error/determine-specific-type.js v="() => ({ __proto__: null })" n=1000000: 1,271,014.61081246

PR:

error/determine-specific-type.jsv="() => 1n"n=1000000: 20,725,839.6296707error/determine-specific-type.jsv="() => true"n=1000000: 140,278,463.97326404error/determine-specific-type.jsv="() => false"n=1000000: 146,549,885.94755125error/determine-specific-type.jsv="() => 2"n=1000000: 49,945,631.68263187error/determine-specific-type.jsv="() => +0"n=1000000: 127,743,303.4086129error/determine-specific-type.jsv="() => -0"n=1000000: 123,600,395.47182535error/determine-specific-type.jsv="() => NaN"n=1000000: 153,607,424.2769161error/determine-specific-type.jsv="() => Infinity"n=1000000: 137,883,393.39304143error/determine-specific-type.jsv="() => \"\""n=1000000: 47,073,709.89790654error/determine-specific-type.jsv="() => \"'\""n=1000000: 8,823,294.789975893error/determine-specific-type.jsv="() => Symbol(\"foo\")"n=1000000: 11,643,085.227570156error/determine-specific-type.jsv="() => function foo() {}"n=1000000: 24,384,259.05368641error/determine-specific-type.jsv="() => null"n=1000000: 223,003,179.3563281error/determine-specific-type.jsv="() => undefined"n=1000000: 203,319,433.73911148error/determine-specific-type.jsv="() => new Array()"n=1000000: 35,201,357.92758342error/determine-specific-type.jsv="() => new BigInt64Array()"n=1000000: 21,667,733.83867111error/determine-specific-type.jsv="() => new BigUint64Array()"n=1000000: 17,649,319.59755033error/determine-specific-type.jsv="() => new Int8Array()"n=1000000: 22,205,003.47563817error/determine-specific-type.jsv="() => new Int16Array()"n=1000000: 21,894,023.991121184error/determine-specific-type.jsv="() => new Int32Array()"n=1000000: 21,752,199.72382537error/determine-specific-type.jsv="() => new Float32Array()"n=1000000: 20,549,685.59494782error/determine-specific-type.jsv="() => new Float64Array()"n=1000000: 21,237,781.66528292error/determine-specific-type.jsv="() => new Uint8Array()"n=1000000: 21,616,408.912246525error/determine-specific-type.jsv="() => new Uint8ClampedArray()"n=1000000: 20,134,607.096901342error/determine-specific-type.jsv="() => new Uint16Array()"n=1000000: 18,759,458.753596798error/determine-specific-type.jsv="() => new Uint32Array()"n=1000000: 21,513,774.5814237error/determine-specific-type.jsv="() => new Date()"n=1000000: 34,218,765.56419165error/determine-specific-type.jsv="() => new Map()"n=1000000: 31,749,019.17755055error/determine-specific-type.jsv="() => new WeakMap()"n=1000000: 23,849,589.985003855error/determine-specific-type.jsv="() => new Object()"n=1000000: 34,675,541.47851863error/determine-specific-type.jsv="() => Promise.resolve(\"foo\")"n=1000000: 34,478,485.66629363error/determine-specific-type.jsv="() => new Set()"n=1000000: 31,878,517.854217436error/determine-specific-type.jsv="() => new WeakSet()"n=1000000: 34,144,917.037412174error/determine-specific-type.jsv="() => ({ __proto__: null })"n=1000000: 1,280,069.176986435

@nodejs-github-botnodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. labels Sep 18, 2023
@UzlopakUzlopak changed the title errors: improve performance of determine-specific-typeerrors: improve performance of determineSpecificTypeSep 18, 2023
@Uzlopak

Copy link
Copy Markdown
ContributorAuthor

@anonrig

can you mark this as author-ready please?

@anonrig

Copy link
Copy Markdown
Member

can you mark this as author-ready please?

The tests are failing @Uzlopak

@Uzlopak

Copy link
Copy Markdown
ContributorAuthor

Oha. Will focus on this when the other PR got merged.

@Uzlopak

Copy link
Copy Markdown
ContributorAuthor

@anonrig
fixed

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 20, 2023
@anonriganonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-benchmark-ci PR that need a benchmark CI run. labels Sep 20, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 20, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Uzlopak

Copy link
Copy Markdown
ContributorAuthor

@anonrig

Can this be merged?

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2023
@anonrig

Copy link
Copy Markdown
Member

We can merge this once the CI finishes.

Pending benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1420/

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Sep 25, 2023
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 26, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/49696
✔ Done loading data for nodejs/node/pull/49696
----------------------------------- PR info ------------------------------------
Title errors: improve performance of determineSpecificType (#49696)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch Uzlopak:improve-determine-specific-type -> nodejs:main
Labels errors, author ready, needs-ci, needs-benchmark-ci, commit-queue-squash
Commits 8
- errors: improve performance of determine-specific-type
- fix lint
- fix lint
- fix function serialization
- Merge branch 'main' into improve-determine-specific-type
- fix invalidArgTypeHelper
- fix test
- Merge branch 'main' into improve-determine-specific-type
Committers 2
- uzlopak - GitHub PR-URL: https://github.com/nodejs/node/pull/49696
Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49696
Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow --------------------------------------------------------------------------------
ℹ This PR was created on Mon, 18 Sep 2023 03:57:53 GMT
✔ Approvals: 2
✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/49696#pullrequestreview-1642824192
✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/49696#pullrequestreview-1636924932
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2023-09-25T19:11:17Z: https://ci.nodejs.org/job/node-test-pull-request/54226/
ℹ Last Benchmark CI on 2023-09-25T19:05:12Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1420/
- Querying data for job/node-test-pull-request/54226/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ No git cherry-pick in progress
✔ No git am in progress
✔ No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
* branch main -> FETCH_HEAD
✔ origin/main is now up-to-date
- Downloading patch for 49696
From https://github.com/nodejs/node
* branch refs/pull/49696/merge -> FETCH_HEAD
✔ Fetched commits as f16f41c5b3cf..3d4b956b2292
--------------------------------------------------------------------------------
Auto-merging lib/internal/errors.js
[main 2ca76c9a83] errors: improve performance of determine-specific-type
Author: uzlopak Date: Mon Sep 18 05:56:37 2023 +0200
3 files changed, 136 insertions(+), 15 deletions(-)
create mode 100644 benchmark/error/determine-specific-type.js
Auto-merging lib/internal/errors.js
[main 6873f4acda] fix lint
Author: uzlopak Date: Mon Sep 18 06:04:59 2023 +0200
1 file changed, 3 insertions(+), 3 deletions(-)
[main 1cdfaf7533] fix lint
Author: uzlopak Date: Mon Sep 18 06:07:13 2023 +0200
1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging lib/internal/errors.js
error: commit 99f6196abb3604436614d0f18a406b14be2cc0b1 is a merge but no -m option was given.
fatal: cherry-pick failed
[main d4c38b51f3] fix function serialization
Author: uzlopak Date: Mon Sep 18 19:00:20 2023 +0200
2 files changed, 34 insertions(+), 5 deletions(-)
✘ Failed to apply patches
https://github.com/nodejs/node/actions/runs/6306204198

@Uzlopak
Uzlopakforce-pushed the improve-determine-specific-type branch from 3d4b956 to d3bd644CompareSeptember 27, 2023 16:35
@anonriganonrig added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 29, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 30, 2023
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 30, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/49696
✔ Done loading data for nodejs/node/pull/49696
----------------------------------- PR info ------------------------------------
Title errors: improve performance of determineSpecificType (#49696)
Author Aras Abbasi (@Uzlopak)
Branch Uzlopak:improve-determine-specific-type -> nodejs:main
Labels errors, author ready, needs-ci, needs-benchmark-ci, commit-queue-squash
Commits 1
- errors: improve performance of determine-specific-type
Committers 1
- uzlopak PR-URL: https://github.com/nodejs/node/pull/49696
Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49696
Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow --------------------------------------------------------------------------------
ℹ This PR was created on Mon, 18 Sep 2023 03:57:53 GMT
✔ Approvals: 2
✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/49696#pullrequestreview-1649996712
✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/49696#pullrequestreview-1636924932
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2023-09-29T11:35:22Z: https://ci.nodejs.org/job/node-test-pull-request/54367/
ℹ Last Benchmark CI on 2023-09-26T00:02:28Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1420/
- Querying data for job/node-test-pull-request/54367/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ No git cherry-pick in progress
✔ No git am in progress
✔ No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
* branch main -> FETCH_HEAD
✔ origin/main is now up-to-date
- Downloading patch for 49696
From https://github.com/nodejs/node
* branch refs/pull/49696/merge -> FETCH_HEAD
✔ Fetched commits as 51f4ff245018..d3bd644908c9
--------------------------------------------------------------------------------
error: cherry-pick is already in progress
hint: try "git cherry-pick (--continue | --abort | --quit)"
fatal: cherry-pick failed
✘ Failed to apply patches
https://github.com/nodejs/node/actions/runs/6359670729

@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 30, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 30, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 0ee9c83 into nodejs:mainSep 30, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 0ee9c83

@Uzlopak
Uzlopak deleted the improve-determine-specific-type branch September 30, 2023 22:01
targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #49696
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@targostargos mentioned this pull request Nov 12, 2023
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
PR-URL: nodejs#49696
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.errorsIssues and PRs related to JavaScript errors originated in Node.js core.needs-benchmark-ciPR that need a benchmark CI run.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Uzlopak@anonrig@nodejs-github-bot@MoLow@aduh95