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.

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

Buffer.byteLength Performance Regression from v18.x to v22.x #50620

Closed
kylo5aby opened this issue Nov 8, 2023 · 0 comments · Fixed by #50621
Closed

Buffer.byteLength Performance Regression from v18.x to v22.x #50620

kylo5aby opened this issue Nov 8, 2023 · 0 comments · Fixed by #50621
Labels
buffer Issues and PRs related to the buffer subsystem. performance Issues and PRs related to the performance of Node.js.

Comments

@kylo5aby
Copy link
Contributor

kylo5aby commented Nov 8, 2023

For Buffer.byteLength(string[, encoding]), when set encoding is buffer:

buffers/buffer-bytelength.js n=4000000 len=16 encoding='buffer'                                                                                   ***    -36.69 %       ±1.78%  ±2.38%  ±3.09%
buffers/buffer-bytelength.js n=4000000 len=2 encoding='buffer'                                                                                    ***    -35.95 %       ±3.34%  ±4.48%  ±5.91%
buffers/buffer-bytelength.js n=4000000 len=256 encoding='buffer'                                                                                  ***    -35.46 %       ±5.18%  ±6.95%  ±9.17%

During my investigation, I found that the performance regression might be due to ArrayBufferView.byteLength.

for the implementation of Buffer.byteLength in lib:

function byteLength(string, encoding) {
  if (typeof string !== 'string') {
    if (isArrayBufferView(string) || isAnyArrayBuffer(string)) {
      return string.byteLength;
    }
    // ...
  }
}

when string.byteLength is not called in either v18.x or v22.x(for example, change to return 0), the regression nearly disappears:

                                                       confidence improvement accuracy (*)   (**)  (***)
buffers/buffer-bytelength-buffer.js n=60000000 len=16         ***     -3.16 %       ±1.25% ±1.66% ±2.16%
buffers/buffer-bytelength-buffer.js n=60000000 len=2                  -1.39 %       ±2.15% ±2.86% ±3.74%
buffers/buffer-bytelength-buffer.js n=60000000 len=256         **     -2.16 %       ±1.28% ±1.72% ±2.25%

Based on the findings, it appears that the handling of ArrayBufferView.byteLength might be the cause of the performance regression observed.

@H4ad H4ad linked a pull request Nov 8, 2023 that will close this issue
@H4ad H4ad added buffer Issues and PRs related to the buffer subsystem. performance Issues and PRs related to the performance of Node.js. labels Nov 8, 2023
nodejs-github-bot pushed a commit that referenced this issue Nov 10, 2023
PR-URL: #50621
Refs: #50620
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos pushed a commit that referenced this issue Nov 11, 2023
PR-URL: #50621
Refs: #50620
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos pushed a commit that referenced this issue Nov 14, 2023
PR-URL: #50621
Refs: #50620
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
PR-URL: #50621
Refs: #50620
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants