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

bootstrap: initialize per-isolate properties of bindings separately #47768

Closed
wants to merge 4 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Apr 28, 2023

src: add per-isolate SetFastMethod and Set[Fast]MethodNoSideEffect

bootstrap: log isolate data info in mksnapshot debug logs

fs: move fs_use_promises_symbol to per-isolate symbols

snapshot: initialize per-isolate properties of bindings separately

This patch moves the initialization of per-isolate properties of
the bindings that are in the embedded snapshot separate from the
initialization of their per-context properties. This is necessary
for workers to share the isolate snapshot with the main thread
and deserialize these properties instead of creating them from
scratch.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 28, 2023

Review requested:

  • @nodejs/startup
  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 28, 2023
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

LGTM

This patch moves the initialization of per-isolate properties of
the bindings that are in the embedded snapshot separate from the
initialization of their per-context properties. This is necessary
for workers to share the isolate snapshot with the main thread
and deserialize these properties instead of creating them from
scratch.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels May 3, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 3, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 2397585...5466bec

nodejs-github-bot pushed a commit that referenced this pull request May 3, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request May 3, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request May 3, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request May 3, 2023
This patch moves the initialization of per-isolate properties of
the bindings that are in the embedded snapshot separate from the
initialization of their per-context properties. This is necessary
for workers to share the isolate snapshot with the main thread
and deserialize these properties instead of creating them from
scratch.

PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
targos pushed a commit that referenced this pull request May 12, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
targos pushed a commit that referenced this pull request May 12, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
targos pushed a commit that referenced this pull request May 12, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
targos pushed a commit that referenced this pull request May 12, 2023
This patch moves the initialization of per-isolate properties of
the bindings that are in the embedded snapshot separate from the
initialization of their per-context properties. This is necessary
for workers to share the isolate snapshot with the main thread
and deserialize these properties instead of creating them from
scratch.

PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
@danielleadams danielleadams added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Jun 29, 2023
targos pushed a commit that referenced this pull request Nov 10, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2023
PR-URL: #47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
PR-URL: nodejs/node#47768
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants