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

src: implement v8 array iteration using the new callback-based API #51758

Closed
wants to merge 3 commits into from

Conversation

joyeecheung
Copy link
Member

src: implement v8 array iteration using the new callback-based API

Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.

src: use callback-based array iteration in Blob

Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.
@nodejs-github-bot nodejs-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 Feb 14, 2024
src/util.h Outdated Show resolved Hide resolved
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

src/util-inl.h Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

joyeecheung added a commit that referenced this pull request Feb 22, 2024
Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.

PR-URL: #51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
joyeecheung added a commit that referenced this pull request Feb 22, 2024
PR-URL: #51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@joyeecheung
Copy link
Member Author

Landed in d51a74a...1a8ae9d

marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.

PR-URL: #51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
PR-URL: #51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@marco-ippolito marco-ippolito added dont-land-on-v21.x PRs that should not land on the v21.x-staging branch and should not be released in v21.x. and removed dont-land-on-v21.x PRs that should not land on the v21.x-staging branch and should not be released in v21.x. labels Feb 26, 2024
@marco-ippolito
Copy link
Member

marco-ippolito commented Feb 26, 2024

@joyeecheung can you backport on 21 please, it seems to fail compilation https://github.com/nodejs/node/actions/runs/8049258088/job/21982250897

@marco-ippolito marco-ippolito added the backport-requested-v21.x PRs awaiting manual backport to the v21.x-staging branch. label Feb 26, 2024
@targos targos added dont-land-on-v21.x PRs that should not land on the v21.x-staging branch and should not be released in v21.x. and removed backport-requested-v21.x PRs awaiting manual backport to the v21.x-staging branch. labels Mar 1, 2024
@targos
Copy link
Member

targos commented Mar 1, 2024

This can't be backported as it relies on V8 API that doesn't exist on v21.x

@targos targos added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Mar 1, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.

PR-URL: nodejs#51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
PR-URL: nodejs#51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
joyeecheung added a commit to joyeecheung/node that referenced this pull request Jun 18, 2024
This backports nodejs#51758
which relies on an V8 API that's not available on v20.x.

Original commit message:

src: implement v8 array iteration using the new callback-based API

Using this to iterate over an array can be faster than calling
Array::Get repeatedly. Local experiment shows that this is faster
once the array size is bigger than 2.

PR-URL: nodejs#51758
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v21.x PRs that should not land on the v21.x-staging branch and should not be released in v21.x. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants