You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@joyeecheung do you have any thoughts/suggestions on the snapshot front?
@sxa I assume that it should be possible to update https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.py so that it generates data in a deterministic manner (maybe sorting them at some point) and it's really just a matter of doing that versus there being some fundamental reason whey it might not be possible?
That would be my assumption yes. The previous issues and PRs suggest this was resolved, so we'd just need to understand what happened and potentially reimplement a fix for it.
Pinging @warpfork for this thread. He's currently heads-down on building Warpforge and has a passion for reproducible builds and I was telling him that there's interest in making progress on this front for Node.js so there might be some potential for collaboration and the production of some tooling to both generate reproducible builds and also document & provide mechanisms for people to do it for themselves without all the pain that usually comes from doing it manually.
So @sxa, meet the most excellent and clever @warpfork; maybe you two should chat.
Thanks @rvagg - I've worked on reproducible build environments for another project, and the Node.js build is close other than those two issues above, so it should just require a little bit of knowledge about the Node processes involved in producing those two things to be able to resolve it, then we can possibly put some more robust things in place for making our builds reproducible by default, which I think should be the aim here.
@warpfork DO you have much experience on non-Linux reproducible build work (I've so far only looked at Linux for Node.js)
1. The production of `debug-support.cc` is nondeterministic - the definitions are not always in the same order within the file. It is generated by https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.py - If I copy in a consistent one the builds can be reproducible.
Instead of a set a dict can be used here (with None values), as since Python 3.7 dict preserves order. Then use for line in dict.fromkeys(lines): to output the lines in order.
Just tried four consecutive builds with the latest code and the debug-support.cc came out the same each time. There have been four V8 version bumps since I last tested (10.1.124.6 ->11.3.244.4) so either I'm lucky today or something in there has made things better, but potentially the fix in the earlier comment will no longer required.
This was tested with ./configure --without-node-snapshot and setting SOURCE_DATE_EPOCH=0 in the environment.
With snapshots enabled the builds are still non-reproducible. due to node_snapshot.cc (Note that node_code_cache.cc referred to in nodejs/node#29108 is no longer a problem on the Linux system being used.
@joyeecheung Would you be able to assist with the reproducibility of the snapshots now? I'm not sure who else might have good knowledge of the snapshot support in Node so anyone else might have to start from scratch.
Thanks for the ping, not sure how I missed the earlier one. I diff'ed the generated snapshot locally and found ~20 lines of differences in the snapshot.cc generated (with --predictable, which we should also set for mksnapshot), most notably the binding data are initialized in different order, not sure why but they are supposed to be initialized in the same order. I'll look into why that happens.
Checking the snapshots again, another source of indeterminism comes from performance data (milestones, time origin etc.), the easiest way to fix it is probably discarding it before snapshot generation. But I'll need to check if/how they should be synchronized.
7 of which coming from binding data's embedder data slot
1 of which coming from the context embedder data slot
We probably need some V8 patches to make it deterministic (my guess is, v8 doesn't actually need to copy the exact values of those slots into the snapshot, they are only there as place holders, so v8 should probably just copy the same amount of 0s for those - still working locally to see if this is correct)
EDIT: we can just return some non-empty data for both BaseObject slots to fix 1. 2 probably need a V8 patch for us to customize how the slots should be serialized. Trying to work out a prototype.
We also add the cached version tag (which includes this bitfield) to the custom snapshot blob, but for the built-in snapshot at least we can remove it since we don't check it for built-in snapshot anyway. But the ones in the code cache would be harder to get rid of as V8 has some concerns about skipping the CPU feature test (see the discussions in https://chromium-review.googlesource.com/c/v8/v8/+/4905290). I wonder if we can convince the upstream to change the bits set in the code cache to be a combination of "CPU features required by this code cache" instead of "CPU features of the platform that compiles this code cache" (the two aren't necessarily the same and at least for now, the former is basically "none" because V8 doesn't include any optimized code in the code cache, though they want to reserve the ability to make it CPU-specific. But at least making it more specific about the actual requirement instead of doing a blind equality check would help our use case).
Oh wait you are already using --without-node-snapshot, then I am not quite sure what else is varying - though according to #3043 (comment) other than libnode it comes from libnghttp2, libicuucx and libucii18n and libicutools
--without-node-snapshot is no longer an option in this release so it builds with it enabled, however the issue (as suspected earlier) was nodejs currently does not build some deps (icu, openssl, brotli, libev, nghttp2, c-ares) deterministically.
I had to package all those myself separately and get those deterministic, then have node build against the system versions instead of in-tree versions, but it works!
Are the reproducibility tests only comparing doing a build twice on identical hardware?
Yes. And it's done on aarch64 (chosen due to the fact we have machines with very high number of cores there which means it's feasible to run the test without ccache without them taking up too much machine time!)
nodejs currently does not build some deps (icu, openssl, brotli, libev, nghttp2, c-ares) deterministically.
That's useful to know - thank you. Although it's interesting that it's only showing up when building on different machines. Do you know if it's definitely a problem for all of those that you mentioned?
The failures are due to ada changes that don't work with gcc 10 (which the job was using). We bumped the minimum supported gcc compiler to 12 for Node.js 23 -- I've edited the reproducibility-test job to swap gcc-10/g++-10 to gcc-12/g++-12 and am running a test build: https://ci.nodejs.org/job/reproducibility-test/102
LGTM and those jobs are running green consistently. I feel it's time we closed this one :-)
ANy subsequent problems can be managed as separate issues now. Thanks everyone, especially @joyeecheung and for the two blog posts on the topic:
Activity
mhdawson commentedon Oct 21, 2022
@joyeecheung do you have any thoughts/suggestions on the snapshot front?
@sxa I assume that it should be possible to update
https://github.com/nodejs/node/blob/main/deps/v8/tools/gen-postmortem-metadata.pyso that it generates data in a deterministic manner (maybe sorting them at some point) and it's really just a matter of doing that versus there being some fundamental reason whey it might not be possible?sxa commentedon Oct 25, 2022
That would be my assumption yes. The previous issues and PRs suggest this was resolved, so we'd just need to understand what happened and potentially reimplement a fix for it.
rvagg commentedon Oct 26, 2022
Pinging @warpfork for this thread. He's currently heads-down on building Warpforge and has a passion for reproducible builds and I was telling him that there's interest in making progress on this front for Node.js so there might be some potential for collaboration and the production of some tooling to both generate reproducible builds and also document & provide mechanisms for people to do it for themselves without all the pain that usually comes from doing it manually.
So @sxa, meet the most excellent and clever @warpfork; maybe you two should chat.
sxa commentedon Oct 26, 2022
Thanks @rvagg - I've worked on reproducible build environments for another project, and the Node.js build is close other than those two issues above, so it should just require a little bit of knowledge about the Node processes involved in producing those two things to be able to resolve it, then we can possibly put some more robust things in place for making our builds reproducible by default, which I think should be the aim here.
@warpfork DO you have much experience on non-Linux reproducible build work (I've so far only looked at Linux for Node.js)
StefanBruens commentedon Dec 6, 2022
One source of indeterminism is here:
https://github.com/nodejs/node/blob/ab064d12b79d14a3d02ba420138cc9d24169a951/deps/v8/tools/gen-postmortem-metadata.py#L717
Instead of a set a dict can be used here (with
Nonevalues), as since Python 3.7 dict preserves order. Then usefor line in dict.fromkeys(lines):to output the lines in order.cclauss commentedon Apr 11, 2023
Is there a todo on this issue or can it be closed?
sxa commentedon Apr 12, 2023
Definitely still work to do and I still plan to look at it.
sxa commentedon Apr 12, 2023
Just tried four consecutive builds with the latest code and the
debug-support.cccame out the same each time. There have been four V8 version bumps since I last tested (10.1.124.6 ->11.3.244.4) so either I'm lucky today or something in there has made things better, but potentially the fix in the earlier comment will no longer required.This was tested with
./configure --without-node-snapshotand settingSOURCE_DATE_EPOCH=0in the environment.With snapshots enabled the builds are still non-reproducible. due to
node_snapshot.cc(Note thatnode_code_cache.ccreferred to in nodejs/node#29108 is no longer a problem on the Linux system being used.sxa commentedon Apr 12, 2023
Looks like the break was between these two commits, so nodejs/node@1faf6f459f likely made it non-reproducible again.
FYI @joyeecheung (PR) @bnoordhuis (PR)
I don't think I have enough knowledge of this code to be able to propose a safe solution here so looking for advice.
sxa commentedon Jun 23, 2023
@joyeecheung Would you be able to assist with the reproducibility of the snapshots now? I'm not sure who else might have good knowledge of the snapshot support in Node so anyone else might have to start from scratch.
joyeecheung commentedon Jun 23, 2023
Thanks for the ping, not sure how I missed the earlier one. I diff'ed the generated snapshot locally and found ~20 lines of differences in the snapshot.cc generated (with
--predictable, which we should also set for mksnapshot), most notably the binding data are initialized in different order, not sure why but they are supposed to be initialized in the same order. I'll look into why that happens.joyeecheung commentedon Jun 23, 2023
With this patch the snapshot.cc difference is down to 13 lines. Still need to figure out the differences in the blob though.
see diff
sxa commentedon Jun 30, 2023
Sounds like good progress - thanks @joyeecheung!
joyeecheung commentedon Jul 8, 2023
Checking the snapshots again, another source of indeterminism comes from performance data (milestones, time origin etc.), the easiest way to fix it is probably discarding it before snapshot generation. But I'll need to check if/how they should be synchronized.
joyeecheung commentedon Jul 9, 2023
With nodejs/node#48702 and nodejs/node#48708 and --predictable the differences are down to 8 places:
We probably need some V8 patches to make it deterministic (my guess is, v8 doesn't actually need to copy the exact values of those slots into the snapshot, they are only there as place holders, so v8 should probably just copy the same amount of 0s for those - still working locally to see if this is correct)EDIT: we can just return some non-empty data for both BaseObject slots to fix 1. 2 probably need a V8 patch for us to customize how the slots should be serialized. Trying to work out a prototype.
60 remaining items
src: add utilities to help debugging reproducibility of snapshots
lrvick commentedon Jul 18, 2024
Are the reproducibility tests only comparing doing a build twice on identical hardware?
Node 22.4.0 is not reproducible in our testing on stagex, when building on two different ryzen machines with different specs.
See my comment here: #3043 (comment)
I tried
CFLAGS="-march=x86-64 -mtune=generic -O2"to try to avoid any cpu-specific optimizations but it seems this was not enough.joyeecheung commentedon Jul 26, 2024
Hmm, I am pretty sure it is related to the CPU features hash V8 adds to the code cache, which is encoded as a bit field including the following properties: https://chromium.googlesource.com/v8/v8/+/7c7c6b475c6edcd7ef863e1f668be3df55c56307/src/codegen/cpu-features.h#15 - so it's not strictly required that the hardware must be identical, but the differences need to be out of the probed set.
We also add the cached version tag (which includes this bitfield) to the custom snapshot blob, but for the built-in snapshot at least we can remove it since we don't check it for built-in snapshot anyway. But the ones in the code cache would be harder to get rid of as V8 has some concerns about skipping the CPU feature test (see the discussions in https://chromium-review.googlesource.com/c/v8/v8/+/4905290). I wonder if we can convince the upstream to change the bits set in the code cache to be a combination of "CPU features required by this code cache" instead of "CPU features of the platform that compiles this code cache" (the two aren't necessarily the same and at least for now, the former is basically "none" because V8 doesn't include any optimized code in the code cache, though they want to reserve the ability to make it CPU-specific. But at least making it more specific about the actual requirement instead of doing a blind equality check would help our use case).
joyeecheung commentedon Jul 26, 2024
Oh wait you are already using
--without-node-snapshot, then I am not quite sure what else is varying - though according to #3043 (comment) other than libnode it comes fromlibnghttp2,libicuucxandlibucii18nandlibicutoolssrc: remove cached data tag from snapshot metadata
src: remove cached data tag from snapshot metadata
lrvick commentedon Aug 26, 2024
Confirmed I am finally able to build nodejs 22.7.0 reproducibly across multiple different systems/cpus:
https://codeberg.org/stagex/stagex/pulls/95/files
--without-node-snapshot is no longer an option in this release so it builds with it enabled, however the issue (as suspected earlier) was nodejs currently does not build some deps (icu, openssl, brotli, libev, nghttp2, c-ares) deterministically.
I had to package all those myself separately and get those deterministic, then have node build against the system versions instead of in-tree versions, but it works!
sxa commentedon Aug 27, 2024
Yes. And it's done on aarch64 (chosen due to the fact we have machines with very high number of cores there which means it's feasible to run the test without ccache without them taking up too much machine time!)
That's useful to know - thank you. Although it's interesting that it's only showing up when building on different machines. Do you know if it's definitely a problem for all of those that you mentioned?
src: remove cached data tag from snapshot metadata
richardlau commentedon Feb 14, 2025
CI build started failing with 4 Feb build. The previous 28 Jan build passed.
The failures are due to ada changes that don't work with gcc 10 (which the job was using). We bumped the minimum supported gcc compiler to 12 for Node.js 23 -- I've edited the reproducibility-test job to swap
gcc-10/g++-10togcc-12/g++-12and am running a test build: https://ci.nodejs.org/job/reproducibility-test/102sxa commentedon Dec 1, 2025
LGTM and those jobs are running green consistently. I feel it's time we closed this one :-)
ANy subsequent problems can be managed as separate issues now. Thanks everyone, especially @joyeecheung and for the two blog posts on the topic: