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.

Sign up for GitHub

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

Jump to bottom

buffer: make indexOf(byte) faster #53455

Merged
merged 1 commit into from
Jun 16, 2024
Merged

buffer: make indexOf(byte) faster #53455

merged 1 commit into from
Jun 16, 2024

Conversation

tniessen
Copy link
Member

Add a V8 fast API implementation for indexOfNumber, which significantly improves the performance of finding a single byte within a buffer.

Local benchmark:

                                                    confidence improvement accuracy (*)   (**)  (***)
buffers/buffer-indexof-number.js n=1000000 value=64        ***     57.69 %       ±5.47% ±7.29% ±9.52%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 1 comparisons, you can thus expect the following amount of false-positive results:
  0.05 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.01 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

Refs: #52993

@tniessen
buffer: make indexOf(byte) faster
669be2d 
Add a V8 fast API implementation for indexOfNumber, which significantly
improves the performance of finding a single byte within a buffer.

Refs: nodejs#52993
@tniessen tniessen added buffer Issues and PRs related to the buffer subsystem. performance Issues and PRs related to the performance of Node.js. labels Jun 14, 2024
@tniessen tniessen requested a review from ronag June 14, 2024 17:16
@tniessen tniessen mentioned this pull request Jun 14, 2024
Merged
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 14, 2024
@tniessen tniessen added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 14, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 14, 2024
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59792/

@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 15, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59818/

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 16, 2024
@nodejs-github-bot nodejs-github-bot merged commit 103623b into nodejs:main Jun 16, 2024
70 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 103623b

@ronag
Copy link
Member

ronag commented Jun 20, 2024

Just a quick question. Any reason why we are not using Uint8Array.prototype.indexOf provided by V8?

targos pushed a commit that referenced this pull request Jun 20, 2024
@tniessen @targos
buffer: make indexOf(byte) faster
dfdc062 
Add a V8 fast API implementation for indexOfNumber, which significantly
improves the performance of finding a single byte within a buffer.

Refs: #52993
PR-URL: #53455
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
EliphazB pushed a commit to EliphazB/node that referenced this pull request Jun 20, 2024
@tniessen @EliphazB
buffer: make indexOf(byte) faster
953c7e9 
Add a V8 fast API implementation for indexOfNumber, which significantly
improves the performance of finding a single byte within a buffer.

Refs: nodejs#52993
PR-URL: nodejs#53455
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@tniessen @bmeck
buffer: make indexOf(byte) faster
e4f66b4 
Add a V8 fast API implementation for indexOfNumber, which significantly
improves the performance of finding a single byte within a buffer.

Refs: nodejs#52993
PR-URL: nodejs#53455
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
@targos targos mentioned this pull request Jun 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemire lemire lemire left review comments

@anonrig anonrig anonrig approved these changes

@ronag ronag ronag approved these changes

@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tniessen @nodejs-github-bot @ronag @lemire @anonrig @VoltrexKeyva