buffer: optimize byteLength for short strings #54345

ronag · 2024-08-12T21:10:41Z

M2

buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='four_bytes'                  5.08 %      ±27.63% ±43.80%  ±75.96%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='one_byte'                    9.92 %      ±43.25% ±67.27% ±112.98%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='three_bytes'                12.47 %      ±31.61% ±49.92%  ±86.02%
buffers/buffer-bytelength-string.js n=4000000 repeat=1 encoding='utf8' type='two_bytes'                  34.02 %      ±52.38% ±76.67% ±116.30%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='four_bytes'          **     82.80 %      ±52.56% ±76.65% ±115.59%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='one_byte'                    5.31 %      ±48.31% ±70.43% ±106.17%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='three_bytes'         **     60.17 %      ±34.55% ±50.81%  ±77.68%
buffers/buffer-bytelength-string.js n=4000000 repeat=2 encoding='utf8' type='two_bytes'           **     68.52 %      ±32.32% ±48.53%  ±76.85%

ronag · 2024-08-12T21:10:48Z

@lemire

ronag · 2024-08-12T21:30:11Z

@lemire couldn't utf8_length_from_latin1 do this?

ronag · 2024-08-12T21:32:47Z

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

codecov · 2024-08-12T22:35:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.09%. Comparing base (298ff4f) to head (e204e9e).
Report is 436 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54345      +/-   ##
==========================================
- Coverage   87.09%   87.09%   -0.01%     
==========================================
  Files         647      648       +1     
  Lines      181816   182234     +418     
  Branches    34884    34964      +80     
==========================================
+ Hits       158360   158717     +357     
- Misses      16764    16791      +27     
- Partials     6692     6726      +34

Files with missing lines	Coverage Δ
src/node_buffer.cc	`75.55% <100.00%> (+0.36%)`	⬆️

... and 53 files with indirect coverage changes

lemire · 2024-08-13T02:50:54Z

@ronag

couldn't utf8_length_from_latin1 do this?

Fair question.

Please see simdutf/simdutf#499

src/node_buffer.cc

lemire · 2024-08-13T02:57:50Z

Long term, it could pay to make simdutf faster in these cases, because we have the runtime dispatching in place. But this PR, as is, should be fine.

PR-URL: nodejs#54345

ronag · 2024-08-13T10:50:50Z

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

nodejs-github-bot · 2024-08-13T11:29:39Z

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

PR-URL: nodejs#54345

nodejs-github-bot · 2024-08-13T14:24:24Z

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

BridgeAR · 2024-08-13T14:40:44Z

src/node_buffer.cc

+  for (; i + 32 <= length; i += 32) {
+    uint64_t v;
+    memcpy(&v, input + i, 8);
+    answer += pop(v);
+    memcpy(&v, input + i + 8, 8);
+    answer += pop(v);
+    memcpy(&v, input + i + 16, 8);
+    answer += pop(v);
+    memcpy(&v, input + i + 24, 8);
+    answer += pop(v);
+  }


Is this really improving the performance? I am surprised that unrolling the loop is beneficial compared to just the lines right afterwards. I would expect the compiler to optimize things like that.

I just took from simdutf @lemire wdyt?

nodejs-github-bot · 2024-08-13T17:00:46Z

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

nodejs-github-bot · 2024-08-14T05:53:38Z

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

PR-URL: nodejs#54345

nodejs-github-bot · 2024-08-14T08:37:48Z

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

nodejs-github-bot · 2024-08-14T10:02:09Z

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

nodejs-github-bot · 2024-08-14T11:45:34Z

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

mcollina

lgtm

nodejs-github-bot · 2024-08-14T21:22:14Z

Landed in 75d25bc

PR-URL: #54345 Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

ronag requested a review from lemire August 12, 2024 21:10

nodejs-github-bot added 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. labels Aug 12, 2024

ronag force-pushed the bytelength-int64 branch 5 times, most recently from 55227cf to 6506c2a Compare August 12, 2024 21:21

lemire mentioned this pull request Aug 13, 2024

optimize utf8_length_from_latin1 for short strings simdutf/simdutf#499

Merged

lemire reviewed Aug 13, 2024

View reviewed changes

src/node_buffer.cc Outdated Show resolved Hide resolved

lemire approved these changes Aug 13, 2024

View reviewed changes

anonrig approved these changes Aug 13, 2024

View reviewed changes

ronag force-pushed the bytelength-int64 branch from 6506c2a to a608ae1 Compare August 13, 2024 06:14

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

110e88b

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from a608ae1 to 110e88b Compare August 13, 2024 06:17

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

b97e1f1

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from 110e88b to b97e1f1 Compare August 13, 2024 06:20

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

0fedfdb

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from b97e1f1 to 0fedfdb Compare August 13, 2024 06:21

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

1d66f25

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from 0fedfdb to 1d66f25 Compare August 13, 2024 06:23

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

6e3ab46

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from 1d66f25 to 6e3ab46 Compare August 13, 2024 06:23

anonrig approved these changes Aug 13, 2024

View reviewed changes

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2024

ronag added a commit to nxtedition/node that referenced this pull request Aug 13, 2024

buffer: optimize byteLength for short strings

dc6d0a4

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from a9325a3 to dc6d0a4 Compare August 13, 2024 13:18

ronag requested a review from benjamingr August 13, 2024 13:19

lemire approved these changes Aug 13, 2024

View reviewed changes

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2024

BridgeAR reviewed Aug 13, 2024

View reviewed changes

ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 14, 2024

buffer: optimize byteLength for short strings

e204e9e

PR-URL: nodejs#54345

ronag force-pushed the bytelength-int64 branch from dc6d0a4 to e204e9e Compare August 14, 2024 08:37

ronag requested a review from mcollina August 14, 2024 14:16

mcollina approved these changes Aug 14, 2024

View reviewed changes

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 14, 2024

nodejs-github-bot merged commit 75d25bc into nodejs:main Aug 14, 2024
49 checks passed

RafaelGSS pushed a commit that referenced this pull request Aug 19, 2024

buffer: optimize byteLength for short strings

15c5f87

PR-URL: #54345 Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

RafaelGSS mentioned this pull request Aug 19, 2024

v22.7.0 proposal #54452

Merged

RafaelGSS pushed a commit that referenced this pull request Aug 21, 2024

buffer: optimize byteLength for short strings

7b641bc

PR-URL: #54345 Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buffer: optimize byteLength for short strings #54345

buffer: optimize byteLength for short strings #54345

ronag commented Aug 12, 2024 •

edited

Loading

ronag commented Aug 12, 2024

ronag commented Aug 12, 2024

ronag commented Aug 12, 2024

codecov bot commented Aug 12, 2024 •

edited

Loading

lemire commented Aug 13, 2024

lemire commented Aug 13, 2024

ronag commented Aug 13, 2024

nodejs-github-bot commented Aug 13, 2024

nodejs-github-bot commented Aug 13, 2024

BridgeAR Aug 13, 2024 •

edited

Loading

ronag Aug 13, 2024

nodejs-github-bot commented Aug 13, 2024

nodejs-github-bot commented Aug 14, 2024 •

edited by ronag

Loading

nodejs-github-bot commented Aug 14, 2024

nodejs-github-bot commented Aug 14, 2024

nodejs-github-bot commented Aug 14, 2024 •

edited by ronag

Loading

mcollina left a comment

nodejs-github-bot commented Aug 14, 2024

buffer: optimize byteLength for short strings #54345

buffer: optimize byteLength for short strings #54345

Conversation

ronag commented Aug 12, 2024 • edited Loading

ronag commented Aug 12, 2024

ronag commented Aug 12, 2024

ronag commented Aug 12, 2024

codecov bot commented Aug 12, 2024 • edited Loading

Codecov Report

lemire commented Aug 13, 2024

lemire commented Aug 13, 2024

ronag commented Aug 13, 2024

nodejs-github-bot commented Aug 13, 2024

nodejs-github-bot commented Aug 13, 2024

BridgeAR Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

ronag Aug 13, 2024

Choose a reason for hiding this comment

nodejs-github-bot commented Aug 13, 2024

nodejs-github-bot commented Aug 14, 2024 • edited by ronag Loading

nodejs-github-bot commented Aug 14, 2024

nodejs-github-bot commented Aug 14, 2024

nodejs-github-bot commented Aug 14, 2024 • edited by ronag Loading

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Aug 14, 2024

ronag commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

BridgeAR Aug 13, 2024 •

edited

Loading

nodejs-github-bot commented Aug 14, 2024 •

edited by ronag

Loading

nodejs-github-bot commented Aug 14, 2024 •

edited by ronag

Loading