[snapshot] Reduce startup snapshot checksum check overhead

Avoid calculating the checksum on every snapshot deserialization.

- Desktop: by default only in release
- Android: once per process

Most snapshot corruptions happen on android devices but there we also
have the highest overhead from calculating the checksum.

Findings doc: https://docs.google.com/document/d/e/2PACX-1vQWdJjrZpTL5VjbP_LHH-qQj-9vcmuLez93WPZhoacJT2bTXfCAdJpbexfJWP9jrAI5ek_416uZE6_W/pub

Bug: v8:12195
Change-Id: Ic7f2f45a9e8ade31c3774a7b659d9c30769e2b44
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3583983Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80260}

src/flags/flag-definitions.h

@@ -1777,9 +1777,10 @@ DEFINE_BOOL(rcs_cpu_time, false,
 DEFINE_IMPLICATION(rcs_cpu_time, rcs)
 
 // snapshot-common.cc
-DEFINE_BOOL(verify_snapshot_checksum, true,
+DEFINE_BOOL(verify_snapshot_checksum, DEBUG_BOOL,
             "Verify snapshot checksums when deserializing snapshots. Enable "
-            "checksum creation and verification for code caches.")
+            "checksum creation and verification for code caches. Enabled by "
+            "default in debug builds and once per process for Android.")
 DEFINE_BOOL(profile_deserialization, false,
             "Print the time it takes to deserialize the snapshot.")
 DEFINE_BOOL(serialization_statistics, false,

src/snapshot/mksnapshot.cc

@@ -85,7 +85,13 @@ class SnapshotFileWriter {
   static void WriteSnapshotFileSuffix(FILE* fp) {
     fprintf(fp, "const v8::StartupData* Snapshot::DefaultSnapshotBlob() {\n");
     fprintf(fp, "  return &blob;\n");
-    fprintf(fp, "}\n\n");
+    fprintf(fp, "}\n");
+    fprintf(fp, "\n");
+    fprintf(
+        fp,
+        "bool Snapshot::ShouldVerifyChecksum(const v8::StartupData* data) {\n");
+    fprintf(fp, "  return true;\n");
+    fprintf(fp, "}\n");
     fprintf(fp, "}  // namespace internal\n");
     fprintf(fp, "}  // namespace v8\n");
   }

src/snapshot/snapshot-empty.cc

@@ -22,5 +22,9 @@ void DisposeNatives() {}
 #endif  // V8_USE_EXTERNAL_STARTUP_DATA
 
 const v8::StartupData* Snapshot::DefaultSnapshotBlob() { return nullptr; }
+bool Snapshot::ShouldVerifyChecksum(const v8::StartupData* data) {
+  return false;
+}
+
 }  // namespace internal
 }  // namespace v8

src/snapshot/snapshot-external.cc

@@ -4,11 +4,11 @@
 
 // Used for building with external snapshots.
 
-#include "src/snapshot/snapshot.h"
-
 #include "src/base/platform/mutex.h"
+#include "src/flags/flags.h"
 #include "src/init/v8.h"  // for V8::Initialize
 #include "src/snapshot/snapshot-source-sink.h"
+#include "src/snapshot/snapshot.h"
 
 #ifndef V8_USE_EXTERNAL_STARTUP_DATA
 #error snapshot-external.cc is used only for the external snapshot build.
@@ -20,6 +20,9 @@ namespace internal {
 
 static base::LazyMutex external_startup_data_mutex = LAZY_MUTEX_INITIALIZER;
 static v8::StartupData external_startup_blob = {nullptr, 0};
+#ifdef V8_TARGET_OS_ANDROID
+static bool external_startup_checksum_verified = false;
+#endif
 
 void SetSnapshotFromFile(StartupData* snapshot_blob) {
   base::MutexGuard lock_guard(external_startup_data_mutex.Pointer());
@@ -29,8 +32,26 @@ void SetSnapshotFromFile(StartupData* snapshot_blob) {
   DCHECK(!external_startup_blob.data);
   DCHECK(Snapshot::SnapshotIsValid(snapshot_blob));
   external_startup_blob = *snapshot_blob;
+#ifdef V8_TARGET_OS_ANDROID
+  external_startup_checksum_verified = false;
+#endif
 }
 
+bool Snapshot::ShouldVerifyChecksum(const v8::StartupData* data) {
+#ifdef V8_TARGET_OS_ANDROID
+  base::MutexGuard lock_guard(external_startup_data_mutex.Pointer());
+  if (data != &external_startup_blob) {
+    return FLAG_verify_snapshot_checksum;
+  }
+  // Verify the external snapshot maximally once per process due to the
+  // additional overhead.
+  if (external_startup_checksum_verified) return false;
+  external_startup_checksum_verified = true;
+  return true;
+#else
+  return FLAG_verify_snapshot_checksum;
+#endif
+}
 
 const v8::StartupData* Snapshot::DefaultSnapshotBlob() {
   base::MutexGuard lock_guard(external_startup_data_mutex.Pointer());

src/snapshot/snapshot.cc

@@ -167,7 +167,10 @@ bool Snapshot::Initialize(Isolate* isolate) {
 
   const v8::StartupData* blob = isolate->snapshot_blob();
   SnapshotImpl::CheckVersion(blob);
-  if (FLAG_verify_snapshot_checksum) CHECK(VerifyChecksum(blob));
+  if (Snapshot::ShouldVerifyChecksum(blob)) {
+    CHECK(VerifyChecksum(blob));
+  }
+
   base::Vector<const byte> startup_data =
       SnapshotImpl::ExtractStartupData(blob);
   base::Vector<const byte> read_only_data =

src/snapshot/snapshot.h

@@ -111,6 +111,7 @@ class Snapshot : public AllStatic {
 
   // To be implemented by the snapshot source.
   static const v8::StartupData* DefaultSnapshotBlob();
+  static bool ShouldVerifyChecksum(const v8::StartupData* data);
 
 #ifdef DEBUG
   static bool SnapshotIsValid(const v8::StartupData* snapshot_blob);

test/cctest/test-serialize.cc

@@ -2715,6 +2715,7 @@ TEST(CodeSerializerFlagChange) {
 }
 
 TEST(CodeSerializerBitFlip) {
+  i::FLAG_verify_snapshot_checksum = true;
   const char* js_source = "function f() { return 'abc'; }; f() + 'def'";
   v8::ScriptCompiler::CachedData* cache = CompileRunAndProduceCache(js_source);