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: support configurable snapshot #50453

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 28, 2023

  • Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file.
  • Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

  • "builder" (SnapshotConfig::builder_script_path): path to the builder script.
  • "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation.

Refs: #42566

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/single-executable
  • @nodejs/startup

@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 Oct 28, 2023
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks great so far!

src/node.h Outdated Show resolved Hide resolved
src/node.h Outdated Show resolved Hide resolved
src/node.h Show resolved Hide resolved
test/embedding/embedtest.cc Outdated Show resolved Hide resolved
@addaleax
Copy link
Member

addaleax commented Nov 16, 2023

@joyeecheung Since the embedder API has documentation here already, we only need to document --build-snapshot-config as well, right? In that case, addaleax/node@ba0b08d is a short addition to the docs that describes it 🙂

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 17, 2023

@addaleax Thanks, although it seems that commit link is not reachable..paste error?

@addaleax
Copy link
Member

@joyeecheung Yup, sorry :/

joyeecheung and others added 2 commits November 27, 2023 13:21
- Add support for --build-snapshot-config which allows passing
  snapshot configurations via a JSON configuration file.
- Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

- "builder" (SnapshotConfig::builder_script_path): path to the
  builder script.
- "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable
  code cache generation.
@joyeecheung joyeecheung marked this pull request as ready for review November 27, 2023 12:28
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023
@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 27, 2023

Thanks. Applied documentation and I think this is now ready for review

@joyeecheung
Copy link
Member Author

cc @nodejs/startup @nodejs/cpp-reviewers

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

@addaleax Do you have time to take another look?

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Sorry, just got back from PTO – looks good!

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 15, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50453
✔  Done loading data for nodejs/node/pull/50453
----------------------------------- PR info ------------------------------------
Title      src: support configurable snapshot (#50453)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:snapshot-config -> nodejs:main
Labels     c++, lib / src, needs-ci, commit-queue-rebase
Commits    2
 - src: support configurable snapshot
 - doc: add documentation for --build-snapshot-config
Committers 1
 - Joyee Cheung 
PR-URL: https://github.com/nodejs/node/pull/50453
Refs: https://github.com/nodejs/node/issues/42566
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Stephen Belanger 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50453
Refs: https://github.com/nodejs/node/issues/42566
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Stephen Belanger 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 28 Oct 2023 20:10:36 GMT
   ✔  Approvals: 3
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/50453#pullrequestreview-1770444150
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/50453#pullrequestreview-1773719524
   ✔  - Stephen Belanger (@Qard): https://github.com/nodejs/node/pull/50453#pullrequestreview-1774806892
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2023-12-14T23:35:32Z: https://ci.nodejs.org/job/node-test-pull-request/56296/
- Querying data for job/node-test-pull-request/56296/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7223088600

@joyeecheung joyeecheung added semver-minor PRs that contain new features and should be released in the next minor version. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 15, 2023
joyeecheung added a commit that referenced this pull request Dec 15, 2023
- Add support for --build-snapshot-config which allows passing
  snapshot configurations via a JSON configuration file.
- Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

- "builder" (SnapshotConfig::builder_script_path): path to the
  builder script.
- "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable
  code cache generation.

PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
joyeecheung pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 15, 2023

CI was green though git node was not able to understand the GitHub CI results again..Landed in 215f4d0...62ca050

@joyeecheung joyeecheung added the notable-change PRs with changes that should be highlighted in changelogs. label Dec 15, 2023
Copy link
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @joyeecheung.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
- Add support for --build-snapshot-config which allows passing
  snapshot configurations via a JSON configuration file.
- Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

- "builder" (SnapshotConfig::builder_script_path): path to the
  builder script.
- "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable
  code cache generation.

PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: TODO
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit to RafaelGSS/node that referenced this pull request Jan 3, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) nodejs#50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) nodejs#50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) nodejs#51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) nodejs#50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) nodejs#51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) nodejs#51246

PR-URL: nodejs#51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 5, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 11, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 15, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246

PR-URL: #51342
Signed-off-by: RafaelGSS <[email protected]>
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
Notable changes:

doc:
  * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) nodejs#50453
lib,src,permission:
  * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) nodejs#50758
net:
  * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) nodejs#51045
src:
  * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) nodejs#50453
src,permission:
  * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) nodejs#51183
timers:
  * (SEMVER-MINOR) export timers.promises (Marco Ippolito) nodejs#51246

PR-URL: nodejs#51342
Signed-off-by: RafaelGSS <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
- Add support for --build-snapshot-config which allows passing
  snapshot configurations via a JSON configuration file.
- Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

- "builder" (SnapshotConfig::builder_script_path): path to the
  builder script.
- "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable
  code cache generation.

PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
codebytere added a commit to electron/electron that referenced this pull request Apr 14, 2024
codebytere added a commit to electron/electron that referenced this pull request Apr 16, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Apr 17, 2024
* chore: bump node in DEPS to v20.12.0

* chore: update build_add_gn_build_files.patch

* chore: update patches

* chore: bump node in DEPS to v20.12.1

* chore: update patches

* build: encode non-ASCII Latin1 characters as one byte in JS2C

nodejs/node#51605

* crypto: use EVP_MD_fetch and cache EVP_MD for hashes

nodejs/node#51034

* chore: update filenames.json

* chore: bump node in DEPS to v20.12.2

* chore: update patches

* src: support configurable snapshot

nodejs/node#50453

* test: remove test-domain-error-types flaky designation

nodejs/node#51717

* src: avoid draining platform tasks at FreeEnvironment

nodejs/node#51290

* chore: fix accidentally deleted v8 dep

* lib: define FormData and fetch etc. in the built-in snapshot

nodejs/node#51598

* chore: rebase on main

* chore: remove stray log

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Cheng <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2024
* chore: bump node in DEPS to v20.13.1

* chore: bump node in DEPS to v20.14.0

* chore: update build_add_gn_build_files.patch

* chore: update patches

* chore: update patches

* build: encode non-ASCII Latin1 characters as one byte in JS2C

nodejs/node#51605

* crypto: use EVP_MD_fetch and cache EVP_MD for hashes

nodejs/node#51034

* chore: update filenames.json

* chore: update patches

* src: support configurable snapshot

nodejs/node#50453

* test: remove test-domain-error-types flaky designation

nodejs/node#51717

* src: avoid draining platform tasks at FreeEnvironment

nodejs/node#51290

* chore: fix accidentally deleted v8 dep

* lib: define FormData and fetch etc. in the built-in snapshot

nodejs/node#51598

* chore: remove stray log

* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL

nodejs/node#52217

* test: skip test for dynamically linked OpenSSL

nodejs/node#52542

* lib, url: add a `windows` option to path parsing

nodejs/node#52509

* src: use dedicated routine to compile function for builtin CJS loader

nodejs/node#52016

* test: mark test as flaky

nodejs/node#52671

* build,tools: add test-ubsan ci

nodejs/node#46297

* src: preload function for Environment

nodejs/node#51539

* deps: update c-ares to 1.28.1

nodejs/node#52285

* chore: fixup

* events: extract addAbortListener for safe internal use

nodejs/node#52081

* module: print location of unsettled top-level await in entry points

nodejs/node#51999

* fs: add stacktrace to fs/promises

nodejs/node#49849

* chore: fixup indices

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Cheng <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.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++. 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. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants