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

sea: generate code cache with deserialized isolate #49226

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

V8 now requires code cache to be compiled from an isolate with the same RO space layout as the one that's going to deserialize the cache, so for a binary built with snapshot, we need to compile the code cache using a deserialized isolate.

Drive-by: ignore "useCodeCache" when "useSnapshot" is true because the compilation would've been done during build time anyway in that case, and print a warning for it.

Refs: nodejs/node-v8#252

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/single-executable

@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. single-executable Issues and PRs related to single-executable applications labels Aug 18, 2023
@joyeecheung
Copy link
Member Author

(Also, just realized that we didn't actually have to create an entire environment to compile the built-in code cache, since we don't need the context snapshot, just the isolate one..)

src/util.h Outdated Show resolved Hide resolved
@joyeecheung joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 18, 2023
V8 now requires code cache to be compiled from an isolate with the
same RO space layout as the one that's going to deserialize the
cache, so for a binary built with snapshot, we need to compile
the code cache using a deserialized isolate.

Drive-by: ignore "useCodeCache" when "useSnapshot" is true because
the compilation would've been done during build time anyway in
that case, and print a warning for it.
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 18, 2023
@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 19, 2023
@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 Aug 22, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 22, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 62b2cf3...5c9daf4

nodejs-github-bot pushed a commit that referenced this pull request Aug 22, 2023
PR-URL: #49226
Refs: nodejs/node-v8#252
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs-github-bot pushed a commit that referenced this pull request Aug 22, 2023
V8 now requires code cache to be compiled from an isolate with the
same RO space layout as the one that's going to deserialize the
cache, so for a binary built with snapshot, we need to compile
the code cache using a deserialized isolate.

Drive-by: ignore "useCodeCache" when "useSnapshot" is true because
the compilation would've been done during build time anyway in
that case, and print a warning for it.

PR-URL: #49226
Refs: nodejs/node-v8#252
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
PR-URL: #49226
Refs: nodejs/node-v8#252
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
V8 now requires code cache to be compiled from an isolate with the
same RO space layout as the one that's going to deserialize the
cache, so for a binary built with snapshot, we need to compile
the code cache using a deserialized isolate.

Drive-by: ignore "useCodeCache" when "useSnapshot" is true because
the compilation would've been done during build time anyway in
that case, and print a warning for it.

PR-URL: #49226
Refs: nodejs/node-v8#252
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs 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-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. single-executable Issues and PRs related to single-executable applications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants