src: remove cached data tag from snapshot metadata

This only served as a preemptive check, but serializing this in the snapshot would make it unreproducible on different hardware. In the current cached data version tag, the V8 version can already be checked as part of the existing Node.js version check. The V8 flags aren't necessarily important for snapshot/code cache mismatches (only a small subset are), and the CPU features currently don't matter, so doing an exact match is stricter than necessary. Removing the check to help making the snapshot more reproducible on different hardware. PR-URL: #54122 Refs: nodejs/build#3043 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
nodejs · Aug 30, 2024 · eef3030 · eef3030
1 parent 1e01bdc
commit eef3030
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 26 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -320,7 +320,6 @@ std::ostream& operator<<(std::ostream& output, const SnapshotMetadata& i) {
          << "  \"" << i.node_version << "\", // node_version\n"
          << "  \"" << i.node_arch << "\", // node_arch\n"
          << "  \"" << i.node_platform << "\", // node_platform\n"
-         << "  " << i.v8_cache_version_tag << ", // v8_cache_version_tag\n"
          << "  " << i.flags << ", // flags\n"
          << "}";
   return output;

diff --git a/src/env.h b/src/env.h
@@ -549,8 +549,6 @@ struct SnapshotMetadata {
   std::string node_version;
   std::string node_arch;
   std::string node_platform;
-  // Result of v8::ScriptCompiler::CachedDataVersionTag().
-  uint32_t v8_cache_version_tag;
   SnapshotFlags flags;
 };
 

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -43,7 +43,6 @@ using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::ObjectTemplate;
-using v8::ScriptCompiler;
 using v8::SnapshotCreator;
 using v8::StartupData;
 using v8::String;
@@ -542,7 +541,6 @@ SnapshotMetadata SnapshotDeserializer::Read() {
   result.node_version = ReadString();
   result.node_arch = ReadString();
   result.node_platform = ReadString();
-  result.v8_cache_version_tag = ReadArithmetic<uint32_t>();
   result.flags = static_cast<SnapshotFlags>(ReadArithmetic<uint32_t>());
 
   if (is_debug) {
@@ -570,9 +568,6 @@ size_t SnapshotSerializer::Write(const SnapshotMetadata& data) {
   written_total += WriteString(data.node_arch);
   Debug("Write Node.js platform %s\n", data.node_platform);
   written_total += WriteString(data.node_platform);
-  Debug("Write V8 cached data version tag %" PRIx32 "\n",
-        data.v8_cache_version_tag);
-  written_total += WriteArithmetic<uint32_t>(data.v8_cache_version_tag);
   Debug("Write snapshot flags %" PRIx32 "\n",
         static_cast<uint32_t>(data.flags));
   written_total += WriteArithmetic<uint32_t>(static_cast<uint32_t>(data.flags));
@@ -697,23 +692,6 @@ bool SnapshotData::Check() const {
     return false;
   }
 
-  if (metadata.type == SnapshotMetadata::Type::kFullyCustomized &&
-      !WithoutCodeCache(metadata.flags)) {
-    uint32_t current_cache_version = v8::ScriptCompiler::CachedDataVersionTag();
-    if (metadata.v8_cache_version_tag != current_cache_version) {
-      // For now we only do this check for the customized snapshots - we know
-      // that the flags we use in the default snapshot are limited and safe
-      // enough so we can relax the constraints for it.
-      fprintf(stderr,
-              "Failed to load the startup snapshot because it was built with "
-              "a different version of V8 or with different V8 configurations.\n"
-              "Expected tag %" PRIx32 ", read %" PRIx32 "\n",
-              current_cache_version,
-              metadata.v8_cache_version_tag);
-      return false;
-    }
-  }
-
   // TODO(joyeecheung): check incompatible Node.js flags.
   return true;
 }
@@ -1180,7 +1158,6 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
                                    per_process::metadata.versions.node,
                                    per_process::metadata.arch,
                                    per_process::metadata.platform,
-                                   v8::ScriptCompiler::CachedDataVersionTag(),
                                    config->flags};
 
   // We cannot resurrect the handles from the snapshot, so make sure that

diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
@@ -19,6 +19,13 @@ test-fs-read-stream-concurrent-reads: PASS, FLAKY
 # https://github.com/nodejs/node/issues/52630
 test-error-serdes: PASS, FLAKY
 
+# Until V8 provides a better way to check for flag mismatch without
+# making the code cache/snapshot unreproducible, disable the test
+# for a preemptive check now. It should idealy fail more gracefully
+# with a better checking mechanism.
+# https://github.com/nodejs/build/issues/3043
+test-snapshot-incompatible: SKIP
+
 [$system==win32]
 
 # Windows on x86