Skip to content

lib: reduce overhead of blob clone - #50110

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
H4ad:perf/cloned_blob
Oct 17, 2023
Merged

lib: reduce overhead of blob clone#50110
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
H4ad:perf/cloned_blob

Conversation

@H4ad

@H4adH4ad commented Oct 10, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Continuing the work started on nodejs/performance#109

 confidence improvement accuracy (*) (**) (***)
blob/clone.js bytes=1024 n=50000 *** 60.74 % ±1.58% ±2.11% ±2.75%
blob/clone.js bytes=1048576 n=50000 *** 50.70 % ±1.84% ±2.46% ±3.23%
blob/clone.js bytes=128 n=50000 *** 62.57 % ±1.43% ±1.91% ±2.49%
Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 3 comparisons, you can thus
expect the following amount of false-positive results:
0.15 false positives, when considering a 5% risk acceptance (*, **, ***),
0.03 false positives, when considering a 1% risk acceptance (**, ***),
0.00 false positives, when considering a 0.1% risk acceptance (***)

/cc @nodejs/performance

@nodejs-github-botnodejs-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 Oct 10, 2023
anonrig
anonrig reviewed Oct 10, 2023
View reviewed changes
Comment threadlib/internal/blob.js Outdated
@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 10, 2023
@anonriganonrig removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 10, 2023
@H4adH4ad changed the title perf: faster blob clonelib: faster blob cloneOct 10, 2023
@H4ad
H4ad marked this pull request as ready for review October 10, 2023 21:40
@tniessen

tniessen commented Oct 11, 2023

Copy link
Copy Markdown
Member

@H4ad Please review our commit message guidelines. As in #49730, this PR does not follow them.

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@github-actionsgithub-actionsBot 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 Oct 11, 2023
@github-actions

github-actionsBot commented Oct 11, 2023

Copy link
Copy Markdown
Contributor
Failed to start CI
node:internal/modules/cjs/loader:1028
const err = new Error(message);
^

Error: Cannot find module 'call-bind'

Require stack:
  • /opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js
    
at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1028:15)
    
at Function.Module._load (node:internal/modules/cjs/loader:873:27)
    
at Module.require (node:internal/modules/cjs/loader:1100:19)
    
at require (node:internal/modules/cjs/helpers:119:18)
    
at Object. (/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js:4:16)
    
at Module._compile (node:internal/modules/cjs/loader:1198:14)
    
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1252:10)
    
at Module.load (node:internal/modules/cjs/loader:1076:32)
    
at Function.Module._load (node:internal/modules/cjs/loader:911:12)
    
at ModuleWrap. (node:internal/modules/esm/translators:169:29) {
    
code: 'MODULE_NOT_FOUND',
    
requireStack: [
    
'/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js'
    
]
    
}
https://github.com/nodejs/node/actions/runs/6479674339

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 11, 2023
@github-actions

github-actionsBot commented Oct 11, 2023

Copy link
Copy Markdown
Contributor
Failed to start CI
node:internal/modules/cjs/loader:1028
const err = new Error(message);
^

Error: Cannot find module 'call-bind'

Require stack:
  • /opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js
    
at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1028:15)
    
at Function.Module._load (node:internal/modules/cjs/loader:873:27)
    
at Module.require (node:internal/modules/cjs/loader:1100:19)
    
at require (node:internal/modules/cjs/helpers:119:18)
    
at Object. (/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js:4:16)
    
at Module._compile (node:internal/modules/cjs/loader:1198:14)
    
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1252:10)
    
at Module.load (node:internal/modules/cjs/loader:1076:32)
    
at Function.Module._load (node:internal/modules/cjs/loader:911:12)
    
at ModuleWrap. (node:internal/modules/esm/translators:169:29) {
    
code: 'MODULE_NOT_FOUND',
    
requireStack: [
    
'/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js'
    
]
    
}
https://github.com/nodejs/node/actions/runs/6479782777

@H4adH4ad changed the title lib: faster blob clonelib: reduce overhead of blob cloneOct 11, 2023
@H4ad

H4ad commented Oct 12, 2023

Copy link
Copy Markdown
MemberAuthor

Someone can add author ready?

@anonriganonrig 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. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Oct 12, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 12, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 12, 2023

Copy link
Copy Markdown
Collaborator

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

@RafaelGSSRafaelGSS added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Oct 15, 2023
RafaelGSS
RafaelGSS approved these changes Oct 15, 2023
View reviewed changes

@RafaelGSSRafaelGSS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

legendecas
legendecas reviewed Oct 15, 2023
View reviewed changes
Comment threadlib/internal/blob.js Outdated
Comment threadlib/internal/blob.js Outdated
@legendecaslegendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2023

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2023

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Oct 17, 2023

Copy link
Copy Markdown
Collaborator

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

@legendecaslegendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 17, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 17, 2023
@nodejs-github-bot
nodejs-github-bot merged commit badba8c into nodejs:mainOct 17, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 17, 2023

Copy link
Copy Markdown
Collaborator

Landed in badba8c

@H4ad
H4ad deleted the perf/cloned_blob branch October 17, 2023 11:56
targos pushed a commit that referenced this pull request Oct 23, 2023
@H4ad@targos
lib: reduce overhead of blob clone
b6021ab
PR-URL: #50110
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@targostargos mentioned this pull request Oct 23, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@legendecaslegendecaslegendecas approved these changes
@QardQardQard approved these changes
@anonriganonriganonrig approved these changes
@RafaelGSSRafaelGSSRafaelGSS approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@H4ad@tniessen@nodejs-github-bot@Qard@anonrig@legendecas@RafaelGSS@mcollina@targos