src: generate snapshot with --predictable by joyeecheung · Pull Request #48749 · nodejs/node

joyeecheung · 2023-07-12T17:19:04Z

To improve determinism of snapshot generation, add --predictable to the V8 flags used to initialize a process launched to generate snapshot. Also add a kGeneratePredictableSnapshot flag to ProcessInitializationFlags for this and moves the configuration of these flags into node::InitializeOncePerProcess() so that it can be shared by embedders.

Refs: nodejs/build#3043

nodejs-github-bot · 2023-07-12T17:19:08Z

Review requested:

@nodejs/startup

src/node.cc

joyeecheung · 2023-07-12T20:22:12Z

This needs https://chromium-review.googlesource.com/c/v8/v8/+/4681766

targos · 2023-07-17T12:42:32Z

https://chromium-review.googlesource.com/c/v8/v8/+/4681766 landed.

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: nodejs#48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d

joyeecheung · 2023-08-14T14:07:15Z

It turns out all the flags implied by --predictable need to be ignored in the flag hash computation too:

--no-concurrent-recompilation
--no-parallel-scavenge
--no-concurrent-marking
--no-concurrent-array-buffer-sweeping
--no-parallel-marking
--no-concurrent-sweeping
--no-parallel-compaction
--no-parallel-pointer-update
--no-memory-reducer
--single-threaded-gc

Although I now wonder if --predictable is actually needed for our current use case (it seems --predictable currently does not actually introduce additional changes to the snapshot?) This might still be needed down the line though, as V8 does use --predictable for their own snapshot. I may come back and pick this up later when it turns out to be necessary

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: nodejs#48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d

joyeecheung · 2023-08-24T17:18:24Z

hmm, looks like this is necessary after all or otherwise we get 2 more differences: nodejs/build#3043 (comment)

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: nodejs#48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d

nodejs-github-bot · 2023-09-07T14:59:52Z

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

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: nodejs#48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d

nodejs-github-bot · 2023-09-18T13:07:20Z

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

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: #48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d PR-URL: #49703 Refs: v8/v8@de9a5de Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>

To improve determinism of snapshot generation, add --predictable to the V8 flags used to initialize a process launched to generate snapshot. Also add a kGeneratePredictableSnapshot flag to ProcessInitializationFlags for this and moves the configuration of these flags into node::InitializeOncePerProcess() so that it can be shared by embedders.

joyeecheung · 2023-09-26T17:13:59Z

Rebased after #49703 is backported and also moved the flag settings into InitializeNodeWithArgsInternal(), together with other V8::SetFlagsFromString calls. We also do not need to set --harmony-import-assertions now because a) that's true by default now and b) we take care of that already in ProcessGlobalArgsInternal()

nodejs-github-bot · 2023-09-26T17:17:47Z

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

nodejs-github-bot · 2023-09-27T12:24:53Z

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

joyeecheung · 2023-09-27T12:25:09Z

@anonrig @richardlau This has been rebased and updated. Can I get some new LGTM please?

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: #48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d PR-URL: #49703 Refs: v8/v8@de9a5de Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>

nodejs-github-bot · 2023-10-09T02:53:20Z

Landed in 387e292

Original commit message: Ignore --predictable when computing flag list hashes This allows reproducible code cache generation. Refs: nodejs#48749 Change-Id: Ib4693de60ddff1fe41d95c10980f763463db3f95 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4681766 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#88943} Refs: v8/v8@b33bf2d PR-URL: nodejs#49703 Refs: v8/v8@de9a5de Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>

To improve determinism of snapshot generation, add --predictable to the V8 flags used to initialize a process launched to generate snapshot. Also add a kGeneratePredictableSnapshot flag to ProcessInitializationFlags for this and moves the configuration of these flags into node::InitializeOncePerProcess() so that it can be shared by embedders. PR-URL: nodejs#48749 Refs: nodejs/build#3043 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

To improve determinism of snapshot generation, add --predictable to the V8 flags used to initialize a process launched to generate snapshot. Also add a kGeneratePredictableSnapshot flag to ProcessInitializationFlags for this and moves the configuration of these flags into node::InitializeOncePerProcess() so that it can be shared by embedders. PR-URL: #48749 Refs: nodejs/build#3043 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

nodejs/node#48749

* chore: bump node in DEPS to v20.10.0 * chore: update feat_initialize_asar_support.patch no code changes; patch just needed an update due to nearby upstream changes Xref: nodejs/node#49986 * chore: update pass_all_globals_through_require.patch no manual changes; patch applied with fuzz Xref: nodejs/node#49657 * chore: update refactor_allow_embedder_overriding_of_internal_fs_calls Xref: nodejs/node#49912 no code changes; patch just needed an update due to nearby upstream changes * chore: update chore_allow_the_node_entrypoint_to_be_a_builtin_module.patch Xref: nodejs/node#49986 minor manual changes needed to sync with upstream change * update fix_expose_the_built-in_electron_module_via_the_esm_loader.patch Xref: nodejs/node#50096 Xref: nodejs/node#50314 in lib/internal/modules/esm/load.js, update the code that checks for `format === 'electron'`. I'd like 👀 on this Xref: nodejs/node#49657 add braces in lib/internal/modules/esm/translators.js to sync with upstream * fix: lazyload fs in esm loaders to apply asar patches * nodejs/node#50127 * nodejs/node#50096 * esm: jsdoc for modules code nodejs/node#49523 * test: set test-cli-node-options as flaky nodejs/node#50296 * deps: update c-ares to 1.20.1 nodejs/node#50082 * esm: bypass CommonJS loader under --default-type=module nodejs/node#49986 * deps: update uvwasi to 0.0.19 nodejs/node#49908 * lib,test: do not hardcode Buffer.kMaxLength nodejs/node#49876 * crypto: account for disabled SharedArrayBuffer nodejs/node#50034 * test: fix edge snapshot stack traces nodejs/node#49659 * src: generate snapshot with --predictable nodejs/node#48749 * chore: fixup patch indices * fs: throw errors from sync branches instead of separate implementations nodejs/node#49913 * crypto: ensure valid point on elliptic curve in SubtleCrypto.importKey nodejs/node#50234 * esm: detect ESM syntax in ambiguous JavaScrip nodejs/node#50096 * fixup! test: fix edge snapshot stack traces * esm: unflag extensionless ES module JavaScript and Wasm in module scope nodejs/node#49974 * [tagged-ptr] Arrowify objects https://chromium-review.googlesource.com/c/v8/v8/+/4705331 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Charles Kerr <[email protected]> Co-authored-by: Shelley Vohr <[email protected]>

To improve determinism of snapshot generation, add --predictable to the V8 flags used to initialize a process launched to generate snapshot. Also add a kGeneratePredictableSnapshot flag to ProcessInitializationFlags for this and moves the configuration of these flags into node::InitializeOncePerProcess() so that it can be shared by embedders. PR-URL: nodejs#48749 Refs: nodejs/build#3043 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

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 Jul 12, 2023

richardlau approved these changes Jul 12, 2023

View reviewed changes

anonrig approved these changes Jul 12, 2023

View reviewed changes

src/node.cc Outdated Show resolved Hide resolved

joyeecheung added the blocked PRs that are blocked by other issues or PRs. label Jul 12, 2023

joyeecheung force-pushed the predictable branch from 04193b0 to 697fef6 Compare July 20, 2023 08:56

joyeecheung removed the blocked PRs that are blocked by other issues or PRs. label Jul 20, 2023

joyeecheung force-pushed the predictable branch from 697fef6 to 1e6968d Compare August 14, 2023 14:19

joyeecheung mentioned this pull request Aug 24, 2023

Reproducible builds on Linux nodejs/build#3043

Closed

joyeecheung force-pushed the predictable branch from 1e6968d to 232afe3 Compare September 7, 2023 14:57

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2023

joyeecheung force-pushed the predictable branch from 232afe3 to df94eb0 Compare September 7, 2023 19:08

joyeecheung force-pushed the predictable branch from df94eb0 to 4cfdafb Compare September 18, 2023 13:03

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

This was referenced Sep 19, 2023

CI Reliability 2023-09-19 nodejs/reliability#670

Open

CI Reliability 2023-09-20 nodejs/reliability#671

Open

CI Reliability 2023-09-21 nodejs/reliability#672

Open

joyeecheung force-pushed the predictable branch from 4cfdafb to d95be40 Compare September 26, 2023 17:11

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023

github-actions bot mentioned this pull request Sep 28, 2023

CI Reliability 2023-09-28 nodejs/reliability#677

Open

19 tasks

richardlau approved these changes Oct 6, 2023

View reviewed changes

legendecas approved these changes Oct 9, 2023

View reviewed changes

legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 9, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 9, 2023

nodejs-github-bot merged commit 387e292 into nodejs:main Oct 9, 2023

RafaelGSS mentioned this pull request Oct 9, 2023

2023-10-17, Version 21.0.0 (Current) #49870

Merged

targos mentioned this pull request Nov 12, 2023

v20.10.0 release proposal #50682

Merged

juanarbol mentioned this pull request Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

codebytere added a commit to electron/electron that referenced this pull request Dec 8, 2023

src: generate snapshot with --predictable

96344de

nodejs/node#48749

codebytere added a commit to electron/electron that referenced this pull request Dec 8, 2023

src: generate snapshot with --predictable

53ed3fa

nodejs/node#48749

Uh oh!

Conversation

joyeecheung commented Jul 12, 2023

Uh oh!

nodejs-github-bot commented Jul 12, 2023

Uh oh!

Uh oh!

joyeecheung commented Jul 12, 2023

Uh oh!

targos commented Jul 17, 2023

Uh oh!

joyeecheung commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Aug 24, 2023

Uh oh!

nodejs-github-bot commented Sep 7, 2023

Uh oh!

nodejs-github-bot commented Sep 18, 2023

Uh oh!

joyeecheung commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 26, 2023

Uh oh!

nodejs-github-bot commented Sep 27, 2023

Uh oh!

joyeecheung commented Sep 27, 2023

Uh oh!

nodejs-github-bot commented Oct 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

joyeecheung commented Aug 14, 2023 •

edited

Loading

joyeecheung commented Sep 26, 2023 •

edited

Loading