buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings #46616

joyeecheung · 2023-02-11T18:32:19Z

Depends on #46619

From a local run: I am a bit surprised by the last few numbers, theoretically there should be no difference for them. Curious about what the benchmark CI would show.

                                                                                              confidence improvement accuracy (*)    (**)   (***)
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='base64' type='four_bytes'                    1.39 %       ±1.78%  ±2.66%  ±4.20%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='base64' type='one_byte'                      0.40 %       ±0.62%  ±0.95%  ±1.56%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='base64' type='three_bytes'                   0.84 %       ±3.13%  ±5.01%  ±8.85%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='base64' type='two_bytes'                    -1.23 %       ±1.66%  ±2.40%  ±3.56%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='four_bytes'               *      1.43 %       ±1.42%  ±2.18%  ±3.58%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='one_byte'               ***     32.48 %       ±1.42%  ±2.05%  ±3.04%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='three_bytes'                    -0.46 %       ±2.74%  ±4.13%  ±6.60%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='two_bytes'                      -0.20 %       ±2.23%  ±3.21%  ±4.74%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='base64' type='four_bytes'                  -0.06 %       ±1.80%  ±2.64%  ±4.01%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='base64' type='one_byte'                     0.47 %       ±1.09%  ±1.63%  ±2.59%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='base64' type='three_bytes'                  0.41 %       ±1.33%  ±2.01%  ±3.19%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='base64' type='two_bytes'                    1.26 %       ±3.57%  ±5.13%  ±7.56%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='utf8' type='four_bytes'            ***      3.23 %       ±0.78%  ±1.17%  ±1.84%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='utf8' type='one_byte'                       0.57 %       ±5.18%  ±7.99% ±13.25%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='utf8' type='three_bytes'           ***     21.65 %       ±0.91%  ±1.31%  ±1.93%
buffers/buffer-bytelength-string.js n=4000000 repeat=16 encoding='utf8' type='two_bytes'             ***     46.84 %       ±2.73%  ±4.37%  ±7.68%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='base64' type='four_bytes'                   -1.38 %       ±2.36%  ±3.67%  ±6.16%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='base64' type='one_byte'                     -0.17 %       ±2.79%  ±4.40%  ±7.58%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='base64' type='three_bytes'                  -0.31 %       ±1.17%  ±1.70%  ±2.56%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='base64' type='two_bytes'                     0.04 %       ±1.84%  ±2.64%  ±3.88%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='four_bytes'                      0.22 %       ±1.54%  ±2.24%  ±3.38%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='one_byte'                        0.64 %       ±1.49%  ±2.14%  ±3.15%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='three_bytes'                     2.56 %       ±2.77%  ±3.98%  ±5.85%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='two_bytes'                *      8.26 %       ±7.40% ±11.39% ±18.79%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='base64' type='four_bytes'                 -0.12 %       ±0.67%  ±0.98%  ±1.47%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='base64' type='one_byte'                    0.40 %       ±1.44%  ±2.08%  ±3.05%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='base64' type='three_bytes'                 0.17 %       ±2.13%  ±3.10%  ±4.66%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='base64' type='two_bytes'                   0.25 %       ±1.49%  ±2.29%  ±3.76%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='utf8' type='four_bytes'           ***      4.59 %       ±0.85%  ±1.27%  ±2.01%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='utf8' type='one_byte'                      0.10 %       ±1.32%  ±1.98%  ±3.15%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='utf8' type='three_bytes'          ***     32.02 %       ±0.44%  ±0.67%  ±1.10%
buffers/buffer-bytelength-string.js n=4000000 repeat=256 encoding='utf8' type='two_bytes'            ***     74.78 %       ±0.73%  ±1.14%  ±1.91%

ronag · 2023-02-13T07:40:54Z

@anonrig, do we even need to go into native functions for short buffers? e.g. 1 byte?

anonrig · 2023-02-13T13:13:29Z

@anonrig, do we even need to go into native functions for short buffers? e.g. 1 byte?

In this context: One byte refers to buffers with one byte string representations such as Ascii text. Using native calls will always be faster.

"This is the way" - Mandalorian

src/node_buffer.cc

addaleax

(re-approving to make sure the github bot is happy)

benchmark/buffers/buffer-bytelength-buffer.js

nodejs-github-bot · 2023-02-19T20:54:17Z

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

nodejs-github-bot · 2023-02-20T10:26:36Z

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

nodejs-github-bot · 2023-02-21T01:37:55Z

Commit Queue failed

- Loading data for nodejs/node/pull/46616 ✔ Done loading data for nodejs/node/pull/46616 ----------------------------------- PR info ------------------------------------ Title buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings (#46616) Author Joyee Cheung (@joyeecheung) Branch joyeecheung:byte-length-one-byte -> nodejs:main Labels c++, lib / src, author ready, needs-ci Commits 5 - src: add SetFastMethodNoSideEffect() - benchmark: split Buffer.byteLength benchmark - buffer: use v8 fast API calls for Buffer.byteLength with sequential o… - fixup! buffer: use v8 fast API calls for Buffer.byteLength with seque… - fixup! benchmark: split Buffer.byteLength benchmark Committers 2 - Joyee Cheung - GitHub PR-URL: https://github.com/nodejs/node/pull/46616 Reviewed-By: Anna Henningsen Reviewed-By: Robert Nagy Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/46616 Reviewed-By: Anna Henningsen Reviewed-By: Robert Nagy Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 11 Feb 2023 18:32:19 GMT ✔ Approvals: 4 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/46616#pullrequestreview-1295966149 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/46616#pullrequestreview-1296865088 ✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/46616#pullrequestreview-1296774718 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/46616#pullrequestreview-1304930817 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-02-20T10:26:36Z: https://ci.nodejs.org/job/node-test-pull-request/49775/ - Querying data for job/node-test-pull-request/49775/ ✔ 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 46616 From https://github.com/nodejs/node * branch refs/pull/46616/merge -> FETCH_HEAD ✔ Fetched commits as b85b5ba10cd7..955cc5215476 -------------------------------------------------------------------------------- Auto-merging src/util.h [main 72bc87f801] src: add SetFastMethodNoSideEffect() Author: Joyee Cheung Date: Sat Feb 11 17:47:46 2023 +0100 3 files changed, 29 insertions(+), 2 deletions(-) [main 63a2cfe78e] benchmark: split Buffer.byteLength benchmark Author: Joyee Cheung Date: Sat Feb 11 17:48:17 2023 +0100 3 files changed, 62 insertions(+), 49 deletions(-) create mode 100644 benchmark/buffers/buffer-bytelength-buffer.js create mode 100644 benchmark/buffers/buffer-bytelength-string.js delete mode 100644 benchmark/buffers/buffer-bytelength.js Auto-merging src/node_external_reference.h [main ce060a1e41] buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings Author: Joyee Cheung Date: Sat Feb 11 17:49:32 2023 +0100 2 files changed, 28 insertions(+), 3 deletions(-) [main e2f3d9f5fb] fixup! buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings Author: Joyee Cheung Date: Mon Feb 13 16:57:01 2023 +0100 1 file changed, 1 insertion(+), 1 deletion(-) [main 24e347e7f3] fixup! benchmark: split Buffer.byteLength benchmark Author: Joyee Cheung Date: Sun Feb 19 21:53:12 2023 +0100 1 file changed, 1 insertion(+), 1 deletion(-) ✔ Patches applied There are 5 commits in the PR. Attempting autorebase. Rebasing (2/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src: add SetFastMethodNoSideEffect()

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

[detached HEAD d7d1c966f8] src: add SetFastMethodNoSideEffect()
Author: Joyee Cheung joyeec9h3@gmail.com
Date: Sat Feb 11 17:47:46 2023 +0100
3 files changed, 29 insertions(+), 2 deletions(-)
Rebasing (3/8)
Rebasing (4/8)
Rebasing (5/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
benchmark: split Buffer.byteLength benchmark

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

[detached HEAD d6ab33d600] benchmark: split Buffer.byteLength benchmark
Author: Joyee Cheung joyeec9h3@gmail.com
Date: Sat Feb 11 17:48:17 2023 +0100
3 files changed, 62 insertions(+), 49 deletions(-)
create mode 100644 benchmark/buffers/buffer-bytelength-buffer.js
create mode 100644 benchmark/buffers/buffer-bytelength-string.js
delete mode 100644 benchmark/buffers/buffer-bytelength.js
Rebasing (6/8)
Rebasing (7/8)
Rebasing (8/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

[detached HEAD 01a8e5500b] buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings
Author: Joyee Cheung joyeec9h3@gmail.com
Date: Sat Feb 11 17:49:32 2023 +0100
2 files changed, 28 insertions(+), 3 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/4228422835

PR-URL: nodejs#46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Use v8 fast API calls for Buffer.byteLength with sequential one-byte strings. PR-URL: nodejs#46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

aduh95 · 2023-02-26T18:03:57Z

Landed in 55dd283...ee1ce18

PR-URL: #46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Use v8 fast API calls for Buffer.byteLength with sequential one-byte strings. PR-URL: #46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Use v8 fast API calls for Buffer.byteLength with sequential one-byte strings. PR-URL: #46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

danielleadams · 2023-04-03T04:08:21Z

This is blocked by #45788

PR-URL: #46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 11, 2023

addaleax approved these changes Feb 12, 2023

View reviewed changes

joyeecheung mentioned this pull request Feb 12, 2023

Buffer.byteLength nodejs/performance#52

Open

ronag approved these changes Feb 13, 2023

View reviewed changes

ronag reviewed Feb 13, 2023

View reviewed changes

src/node_buffer.cc Outdated Show resolved Hide resolved

ronag reviewed Feb 13, 2023

View reviewed changes

src/node_buffer.cc Show resolved Hide resolved

joyeecheung marked this pull request as ready for review February 13, 2023 15:56

addaleax approved these changes Feb 13, 2023

View reviewed changes

ronag reviewed Feb 13, 2023

View reviewed changes

benchmark/buffers/buffer-bytelength-buffer.js Outdated Show resolved Hide resolved

anonrig approved these changes Feb 14, 2023

View reviewed changes

ronag approved these changes Feb 14, 2023

View reviewed changes

github-actions bot mentioned this pull request Feb 20, 2023

CI Reliability 2023-02-20 nodejs/reliability#501

Open

24 tasks

jasnell approved these changes Feb 20, 2023

View reviewed changes

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 20, 2023

joyeecheung mentioned this pull request Feb 20, 2023

parallel/test-runner-reporters is flaking #46747

Closed

github-actions bot mentioned this pull request Feb 21, 2023

CI Reliability 2023-02-21 nodejs/reliability#502

Open

26 tasks

anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2023

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 21, 2023

joyeecheung added 2 commits February 26, 2023 19:02

benchmark: split Buffer.byteLength benchmark

43ddf76

PR-URL: nodejs#46616 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com>

aduh95 force-pushed the byte-length-one-byte branch from 955cc52 to ee1ce18 Compare February 26, 2023 18:03

aduh95 merged commit ee1ce18 into nodejs:main Feb 26, 2023

targos mentioned this pull request Mar 14, 2023

v19.8.0 release proposal #47087

Merged

danielleadams added the backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. label Apr 3, 2023

danielleadams mentioned this pull request Apr 11, 2023

v18.16.0 release proposal #47502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings #46616

buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings #46616

joyeecheung commented Feb 11, 2023 •

edited

ronag commented Feb 13, 2023

anonrig commented Feb 13, 2023

addaleax left a comment

nodejs-github-bot commented Feb 19, 2023

nodejs-github-bot commented Feb 20, 2023

nodejs-github-bot commented Feb 21, 2023

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

Successfully rebased and updated refs/heads/main.

aduh95 commented Feb 26, 2023

danielleadams commented Apr 3, 2023

buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings #46616

buffer: use v8 fast API calls for Buffer.byteLength with sequential one-byte strings #46616

Conversation

joyeecheung commented Feb 11, 2023 • edited

ronag commented Feb 13, 2023

anonrig commented Feb 13, 2023

addaleax left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Feb 19, 2023

nodejs-github-bot commented Feb 20, 2023

nodejs-github-bot commented Feb 21, 2023

PR-URL: #46616 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Yagiz Nizipli yagiz@nizipli.com Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Yagiz Nizipli yagiz@nizipli.com Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Yagiz Nizipli yagiz@nizipli.com Reviewed-By: James M Snell jasnell@gmail.com

Successfully rebased and updated refs/heads/main.

aduh95 commented Feb 26, 2023

danielleadams commented Apr 3, 2023

joyeecheung commented Feb 11, 2023 •

edited

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com

PR-URL: #46616
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: James M Snell jasnell@gmail.com