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: use native copy impl #54087

Merged
merged 1 commit into from
Aug 2, 2024
Merged

buffer: use native copy impl #54087

merged 1 commit into from
Aug 2, 2024

Conversation

ronag
Copy link
Member

@ronag ronag commented Jul 28, 2024 •
edited
Loading

Apple M2 Pro

buffers/buffer-copy.js n=6000000 partial='false' bytes=0                    -0.53 %       ±6.78%  ±9.02% ±11.74%
buffers/buffer-copy.js n=6000000 partial='false' bytes=128          ***     30.37 %      ±10.39% ±13.88% ±18.19%
buffers/buffer-copy.js n=6000000 partial='false' bytes=32768                 2.73 %       ±3.96%  ±5.27%  ±6.86%
buffers/buffer-copy.js n=6000000 partial='false' bytes=8            ***     73.09 %      ±10.43% ±13.89% ±18.08%
buffers/buffer-copy.js n=6000000 partial='true' bytes=0                     -1.66 %       ±7.78% ±10.35% ±13.47%
buffers/buffer-copy.js n=6000000 partial='true' bytes=128           ***    327.50 %      ±23.05% ±31.00% ±41.01%
buffers/buffer-copy.js n=6000000 partial='true' bytes=32768           *      4.50 %       ±4.34%  ±5.78%  ±7.55%
buffers/buffer-copy.js n=6000000 partial='true' bytes=8             ***    311.90 %      ±33.99% ±45.75% ±60.62%

Benchmark CI

10:33:07 buffers/buffer-copy.js n=6000000 partial='false' bytes=0                     0.56 %       ±1.11%  ±1.48%  ±1.95%
10:33:07 buffers/buffer-copy.js n=6000000 partial='false' bytes=128          ***     34.10 %       ±0.81%  ±1.08%  ±1.41%
10:33:07 buffers/buffer-copy.js n=6000000 partial='false' bytes=32768        ***     -1.80 %       ±0.84%  ±1.12%  ±1.47%
10:33:07 buffers/buffer-copy.js n=6000000 partial='false' bytes=8            ***     38.98 %       ±1.00%  ±1.34%  ±1.75%
10:33:07 buffers/buffer-copy.js n=6000000 partial='true' bytes=0                     -0.44 %       ±0.81%  ±1.08%  ±1.41%
10:33:07 buffers/buffer-copy.js n=6000000 partial='true' bytes=128           ***    256.54 %       ±1.31%  ±1.75%  ±2.31%
10:33:07 buffers/buffer-copy.js n=6000000 partial='true' bytes=32768         ***     35.39 %      ±11.48% ±15.46% ±20.49%
10:33:07 buffers/buffer-copy.js n=6000000 partial='true' bytes=8             ***    249.26 %       ±1.67%  ±2.24%  ±2.95%

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels Jul 28, 2024
@ronag
Copy link
Member Author

ronag commented Jul 28, 2024

@tniessen @anonrig @nodejs/performance @nodejs/buffer

@ronag ronag force-pushed the buffer-copy branch 3 times, most recently from d525f28 to 509b66d Compare July 28, 2024 21:39
@anonrig anonrig 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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jul 28, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 28, 2024
@github-actions GitHub Actions
Copy link
Contributor

Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 400 Bad Request
https://github.com/nodejs/node/actions/runs/10134974481

@ronag ronag force-pushed the buffer-copy branch 2 times, most recently from 147b14d to ce07761 Compare July 28, 2024 22:29
@jakecastelli jakecastelli added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 29, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 29, 2024
@github-actions GitHub Actions
Copy link
Contributor

Failed to start CI
   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ℹ  request-ci label was added by a Collaborator after the last push event.
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 400 Bad Request
https://github.com/nodejs/node/actions/runs/10137136972

@targos targos mentioned this pull request Jul 29, 2024
Closed
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@ronag

This comment was marked as resolved.

Sign in to view
@ronag

This comment was marked as outdated.

Sign in to view
@ronag ronag force-pushed the buffer-copy branch 3 times, most recently from 68cc51b to 7328b6f Compare July 29, 2024 05:58
@aduh95
Copy link
Contributor

aduh95 commented Jul 31, 2024

@aduh95 ?

https://openjs-foundation.slack.com/archives/C03BJP63CH0/p1722428074301359

@ronag
Copy link
Member Author

ronag commented Jul 31, 2024 •
edited
Loading

@aduh95 I can't follow that link. Why did you remove the commit-queue label?

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95
Copy link
Contributor

aduh95 commented Jul 31, 2024

Because of the Windows failure

@ronag
Copy link
Member Author

ronag commented Jul 31, 2024

@aduh95 seems unrelated are there known issues with the windows do?

@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 31, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 31, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 2, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 2, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 2, 2024
@nodejs-github-bot nodejs-github-bot merged commit 76c8ba9 into nodejs:main Aug 2, 2024
65 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 76c8ba9

targos pushed a commit that referenced this pull request Aug 14, 2024
@ronag @targos
buffer: use native copy impl
9f8f26e 
PR-URL: #54087
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Daniel Lemire <[email protected]>
RafaelGSS added a commit that referenced this pull request Aug 19, 2024
@RafaelGSS
2024-08-20, Version 22.7.0 (Current)
5414225 
Notable changes:

buffer:
  * use fast API for writing one-byte strings (Robert Nagy) #54311
  * optimize createFromString (Robert Nagy) #54324
  * use native copy impl (Robert Nagy) #54087
inspector:
  * (SEMVER-MINOR) support `Network.loadingFailed` event (Kohei Ueno) #54246
lib:
  * (SEMVER-MINOR) rewrite AsyncLocalStorage without async_hooks (Stephen Belanger) #48528
module:
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619

PR-URL: TODO
RafaelGSS added a commit that referenced this pull request Aug 19, 2024
@RafaelGSS
2024-08-20, Version 22.7.0 (Current)
69b8bbe 
Notable changes:

buffer:
  * use fast API for writing one-byte strings (Robert Nagy) #54311
  * optimize createFromString (Robert Nagy) #54324
  * use native copy impl (Robert Nagy) #54087
inspector:
  * (SEMVER-MINOR) support `Network.loadingFailed` event (Kohei Ueno) #54246
lib:
  * (SEMVER-MINOR) rewrite AsyncLocalStorage without async_hooks (Stephen Belanger) #48528
module:
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619

PR-URL: #54452
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
Merged
RafaelGSS added a commit that referenced this pull request Aug 20, 2024
@RafaelGSS
2024-08-20, Version 22.7.0 (Current)
56a5662 
Notable changes:

buffer:
  * use fast API for writing one-byte strings (Robert Nagy) #54311
  * optimize createFromString (Robert Nagy) #54324
  * use native copy impl (Robert Nagy) #54087
inspector:
  * (SEMVER-MINOR) support `Network.loadingFailed` event (Kohei Ueno) #54246
lib:
  * (SEMVER-MINOR) rewrite AsyncLocalStorage without async_hooks (Stephen Belanger) #48528
module:
  * (SEMVER-MINOR) add --experimental-transform-types flag (Marco Ippolito) #54283
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619

PR-URL: #54452
RafaelGSS added a commit that referenced this pull request Aug 21, 2024
@RafaelGSS
2024-08-22, Version 22.7.0 (Current)
65eff1e 
Notable changes:

buffer:
  * use fast API for writing one-byte strings (Robert Nagy) #54311
  * optimize createFromString (Robert Nagy) #54324
  * use native copy impl (Robert Nagy) #54087
inspector:
  * (SEMVER-MINOR) support `Network.loadingFailed` event (Kohei Ueno) #54246
lib:
  * (SEMVER-MINOR) rewrite AsyncLocalStorage without async_hooks (Stephen Belanger) #48528
module:
  * (SEMVER-MINOR) add --experimental-transform-types flag (Marco Ippolito) #54283
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619

PR-URL: #54452
RafaelGSS added a commit that referenced this pull request Aug 22, 2024
@RafaelGSS
2024-08-22, Version 22.7.0 (Current)
4d5c885 
Notable changes:

buffer:
  * use fast API for writing one-byte strings (Robert Nagy) #54311
  * optimize createFromString (Robert Nagy) #54324
  * use native copy impl (Robert Nagy) #54087
inspector:
  * (SEMVER-MINOR) support `Network.loadingFailed` event (Kohei Ueno) #54246
lib:
  * (SEMVER-MINOR) rewrite AsyncLocalStorage without async_hooks (Stephen Belanger) #48528
module:
  * (SEMVER-MINOR) add --experimental-transform-types flag (Marco Ippolito) #54283
  * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619

PR-URL: #54452
@dlarocque dlarocque mentioned this pull request Aug 23, 2024
Closed
@ronag ronag mentioned this pull request Aug 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemire lemire lemire approved these changes

@benjamingr benjamingr benjamingr approved these changes

@anonrig anonrig anonrig approved these changes

@mcollina mcollina Awaiting requested review from mcollina

@targos targos Awaiting requested review from targos

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. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ronag @nodejs-github-bot @aduh95 @mcollina @lemire @benjamingr @anonrig @targos @jakecastelli