Revert "Unconditionally enable snapshot builds and remove 'v8_use_snapshot'"

This reverts commit 1c56974f.

Reason for revert: Causes several bots to timeout, e.g. https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20debug/27945

Original change's description:
> Unconditionally enable snapshot builds and remove 'v8_use_snapshot'
> 
> This CL removes 'v8_use_snapshot' and the usages of the implied
> V8_USE_SNAPSHOT define. One test runner unittest was updated to use the
> "asan" variant instead of the now obsolete "no_snap" variant.
> 
> Related chromium CL: https://crrev.com/c/1796325.
> 
> Bug: v8:8531
> Change-Id: I5da7c9f8e9110fe7bc0f4e4f821bcb7f7d98f927
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1784282
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Reviewed-by: Tamer Tas <tmrts@chromium.org>
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#64290}

TBR=thakis@chromium.org,machenbach@chromium.org,mstarzinger@chromium.org,jgruber@chromium.org,tmrts@chromium.org,szuend@chromium.org

Change-Id: I4024d818877e534b9f7908a2d14f33dca35b5924
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8531
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1862572Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64293}

BUILD.gn

@@ -243,7 +243,7 @@ if (v8_enable_snapshot_native_code_counters == "") {
   v8_enable_snapshot_native_code_counters = v8_enable_debugging_features
 }
 if (v8_enable_shared_ro_heap == "") {
-  v8_enable_shared_ro_heap = !v8_enable_pointer_compression
+  v8_enable_shared_ro_heap = !v8_enable_pointer_compression && v8_use_snapshot
 }
 if (v8_enable_fast_torque == "") {
   v8_enable_fast_torque = v8_enable_fast_mksnapshot
@@ -261,10 +261,14 @@ assert(v8_current_cpu != "x86" || !v8_untrusted_code_mitigations,
 
 assert(!v8_enable_lite_mode || v8_enable_embedded_builtins,
        "Lite mode requires embedded builtins")
+assert(!v8_enable_lite_mode || v8_use_snapshot,
+       "Lite mode requires a snapshot build")
 
 assert(
     !v8_enable_pointer_compression || !v8_enable_shared_ro_heap,
     "Pointer compression is not supported with shared read-only heap enabled")
+assert(v8_use_snapshot || !v8_enable_shared_ro_heap,
+       "Shared read-only heap requires snapshot")
 
 assert(v8_extra_library_files == [],
        "v8_extra_library_files is no longer supported. Consider implementing " +
@@ -443,8 +447,11 @@ config("features") {
   if (v8_enable_handle_zapping) {
     defines += [ "ENABLE_HANDLE_ZAPPING" ]
   }
-  if (v8_enable_snapshot_native_code_counters) {
-    defines += [ "V8_SNAPSHOT_NATIVE_CODE_COUNTERS" ]
+  if (v8_use_snapshot) {
+    defines += [ "V8_USE_SNAPSHOT" ]
+    if (v8_enable_snapshot_native_code_counters) {
+      defines += [ "V8_SNAPSHOT_NATIVE_CODE_COUNTERS" ]
+    }
   }
   if (v8_enable_single_generation) {
     defines += [ "V8_ENABLE_SINGLE_GENERATION" ]
@@ -1328,29 +1335,31 @@ template("run_mksnapshot") {
   }
 }
 
-run_mksnapshot("default") {
-  args = []
-  if (v8_enable_embedded_builtins) {
-    embedded_variant = "Default"
-  }
-}
-if (emit_builtins_as_inline_asm) {
-  asm_to_inline_asm("default") {
+if (v8_use_snapshot) {
+  run_mksnapshot("default") {
     args = []
-  }
-}
-if (v8_use_multi_snapshots) {
-  run_mksnapshot("trusted") {
-    args = [ "--no-untrusted-code-mitigations" ]
     if (v8_enable_embedded_builtins) {
-      embedded_variant = "Trusted"
+      embedded_variant = "Default"
     }
   }
   if (emit_builtins_as_inline_asm) {
-    asm_to_inline_asm("trusted") {
+    asm_to_inline_asm("default") {
       args = []
     }
   }
+  if (v8_use_multi_snapshots) {
+    run_mksnapshot("trusted") {
+      args = [ "--no-untrusted-code-mitigations" ]
+      if (v8_enable_embedded_builtins) {
+        embedded_variant = "Trusted"
+      }
+    }
+    if (emit_builtins_as_inline_asm) {
+      asm_to_inline_asm("trusted") {
+        args = []
+      }
+    }
+  }
 }
 
 action("v8_dump_build_config") {
@@ -1380,6 +1389,7 @@ action("v8_dump_build_config") {
     "v8_enable_i18n_support=$v8_enable_i18n_support",
     "v8_enable_verify_predictable=$v8_enable_verify_predictable",
     "v8_target_cpu=\"$v8_target_cpu\"",
+    "v8_use_snapshot=$v8_use_snapshot",
     "v8_enable_embedded_builtins=$v8_enable_embedded_builtins",
     "v8_enable_verify_csa=$v8_enable_verify_csa",
     "v8_enable_lite_mode=$v8_enable_lite_mode",
@@ -1400,14 +1410,20 @@ action("v8_dump_build_config") {
 #
 
 source_set("v8_maybe_snapshot") {
-  if (v8_use_external_startup_data) {
+  if (v8_use_snapshot && v8_use_external_startup_data) {
     public_deps = [
       ":v8_external_snapshot",
     ]
-  } else {
+  } else if (v8_use_snapshot) {
     public_deps = [
       ":v8_snapshot",
     ]
+  } else {
+    # Ignore v8_use_external_startup_data setting if no snapshot is used.
+    public_deps = [
+      ":v8_init",
+      ":v8_nosnapshot",
+    ]
   }
 }
 
@@ -1432,7 +1448,7 @@ v8_source_set("v8_nosnapshot") {
   configs = [ ":internal_config" ]
 }
 
-if (!v8_use_external_startup_data) {
+if (v8_use_snapshot && !v8_use_external_startup_data) {
   v8_source_set("v8_snapshot") {
     # Only targets in this file and the top-level visibility target can
     # depend on this.
@@ -1474,7 +1490,7 @@ if (!v8_use_external_startup_data) {
   }
 }
 
-if (v8_use_external_startup_data) {
+if (v8_use_snapshot && v8_use_external_startup_data) {
   v8_source_set("v8_external_snapshot") {
     visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -3898,7 +3914,7 @@ if (current_toolchain == v8_generator_toolchain) {
   }
 }
 
-if (current_toolchain == v8_snapshot_toolchain) {
+if (v8_use_snapshot && current_toolchain == v8_snapshot_toolchain) {
   v8_executable("mksnapshot") {
     visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -4174,10 +4190,13 @@ if (is_component_build) {
       ":torque_ls_base",
       ":v8_base",
       ":v8_headers",
-      ":v8_initializers",
       ":v8_maybe_snapshot",
     ]
 
+    if (v8_use_snapshot) {
+      public_deps += [ ":v8_initializers" ]
+    }
+
     configs = [ ":internal_config" ]
 
     public_configs = [ ":external_config" ]
@@ -4199,10 +4218,13 @@ if (is_component_build) {
       ":torque_base",
       ":torque_ls_base",
       ":v8_base",
-      ":v8_initializers",
       ":v8_maybe_snapshot",
     ]
 
+    if (v8_use_snapshot) {
+      public_deps += [ ":v8_initializers" ]
+    }
+
     public_configs = [ ":external_config" ]
   }
 }

gni/v8.gni

@@ -35,6 +35,10 @@ declare_args() {
   # on platform and embedder level.
   v8_enable_raw_heap_snapshots = false
 
+  # Enable the snapshot feature, for fast context creation.
+  # https://v8.dev/blog/custom-startup-snapshots
+  v8_use_snapshot = true
+
   # Enable several snapshots side-by-side (e.g. default and for trusted code).
   v8_use_multi_snapshots = false
 
@@ -62,8 +66,8 @@ declare_args() {
 
 if (v8_use_external_startup_data == "") {
   # If not specified as a gn arg, use external startup data by default if
-  # we're not on ios.
-  v8_use_external_startup_data = !is_ios
+  # a snapshot is used and if we're not on ios.
+  v8_use_external_startup_data = v8_use_snapshot && !is_ios
 }
 
 if (v8_use_multi_snapshots) {

infra/mb/mb_config.pyl

@@ -829,9 +829,9 @@
       'gn_args': 'v8_use_external_startup_data=false',
     },
 
-    # TODO(https://crbug.com/v8/8531): Remove this config and all bots, since
-    #                                  no-snapshot was deprecated and removed.
-    'v8_snapshot_none': {},
+    'v8_snapshot_none': {
+      'gn_args': 'v8_use_snapshot=false',
+    },
 
     'v8_verify_heap': {
       'gn_args': 'v8_enable_verify_heap=true',

src/heap/heap-inl.h

@@ -209,7 +209,9 @@ AllocationResult Heap::AllocateRaw(int size_in_bytes, AllocationType type,
   } else if (AllocationType::kMap == type) {
     allocation = map_space_->AllocateRawUnaligned(size_in_bytes);
   } else if (AllocationType::kReadOnly == type) {
+#ifdef V8_USE_SNAPSHOT
     DCHECK(isolate_->serializer_enabled());
+#endif
     DCHECK(!large_object);
     DCHECK(CanAllocateInReadOnlySpace());
     DCHECK_EQ(AllocationOrigin::kRuntime, origin);

test/cctest/test-debug.cc

@@ -1031,6 +1031,8 @@ TEST(BreakPointBuiltinNewContext) {
   i::Handle<i::BreakPoint> bp;
 
 // === Test builtin from a new context ===
+// This does not work with no-snapshot build.
+#ifdef V8_USE_SNAPSHOT
   break_point_hit_count = 0;
   builtin = CompileRun("String.prototype.repeat").As<v8::Function>();
   CompileRun("'a'.repeat(10)");
@@ -1056,6 +1058,7 @@ TEST(BreakPointBuiltinNewContext) {
     CompileRun("'b'.repeat(10)");
     CHECK_EQ(2, break_point_hit_count);
   }
+#endif
 
   v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded();

test/mkgrokdump/mkgrokdump.cc

@@ -26,9 +26,10 @@ static const char* kHeader =
     "\n"
     "# List of known V8 instance types.\n";
 
+// Non-snapshot builds allocate objects to different places.
 // Debug builds emit debug code, affecting code object sizes.
 // Embedded builtins cause objects to be allocated in different locations.
-#if defined(V8_EMBEDDED_BUILTINS) && !defined(DEBUG)
+#if defined(V8_EMBEDDED_BUILTINS) && defined(V8_USE_SNAPSHOT) && !defined(DEBUG)
 static const char* kBuild = "shipping";
 #else
 static const char* kBuild = "non-shipping";

test/unittests/logging/counters-unittest.cc

@@ -149,11 +149,16 @@ class SnapshotNativeCounterTest : public TestWithNativeContextAndCounters {
   SnapshotNativeCounterTest() {}
 
   bool SupportsNativeCounters() const {
+#ifdef V8_USE_SNAPSHOT
 #ifdef V8_SNAPSHOT_NATIVE_CODE_COUNTERS
     return true;
 #else
     return false;
 #endif  // V8_SNAPSHOT_NATIVE_CODE_COUNTERS
+#else
+    // If we do not have a snapshot then we rely on the runtime option.
+    return internal::FLAG_native_code_counters;
+#endif  // V8_USE_SNAPSHOT
   }
 
 #define SC(name, caption)                                        \

tools/testrunner/base_runner.py

@@ -194,9 +194,7 @@ class BuildConfig(object):
     self.is_full_debug = build_config['is_full_debug']
     self.msan = build_config['is_msan']
     self.no_i18n = not build_config['v8_enable_i18n_support']
-    # TODO(https://crbug.com/v8/8531)
-    # 'v8_use_snapshot' was removed, 'no_snap' can be removed as well.
-    self.no_snap = False
+    self.no_snap = not build_config['v8_use_snapshot']
     self.predictable = build_config['v8_enable_verify_predictable']
     self.tsan = build_config['is_tsan']
     # TODO(machenbach): We only have ubsan not ubsan_vptr.

tools/unittests/run_tests_test.py

@@ -338,7 +338,7 @@ class SystemTest(unittest.TestCase):
           basedir, dcheck_always_on=True, is_asan=True, is_cfi=True,
           is_msan=True, is_tsan=True, is_ubsan_vptr=True, target_cpu='x86',
           v8_enable_i18n_support=False, v8_target_cpu='x86',
-          v8_enable_embedded_builtins=False,
+          v8_use_snapshot=False, v8_enable_embedded_builtins=False,
           v8_enable_verify_csa=False, v8_enable_lite_mode=False,
           v8_enable_pointer_compression=False)
       result = run_tests(
@@ -355,6 +355,7 @@ class SystemTest(unittest.TestCase):
           'dcheck_always_on\n'
           'msan\n'
           'no_i18n\n'
+          'no_snap\n'
           'tsan\n'
           'ubsan_vptr\n'
           '>>> Running tests for ia32.release')
@@ -571,7 +572,7 @@ class SystemTest(unittest.TestCase):
     variants.
     """
     with temp_base() as basedir:
-      override_build_config(basedir, is_asan=True)
+      override_build_config(basedir, v8_use_snapshot=False)
       result = run_tests(
           basedir,
           '--mode=Release',

tools/unittests/testdata/testroot1/test/sweet/sweet.status

@@ -29,7 +29,7 @@
   'regress/*': [CRASH],
 }],
 
-['asan', {
+['no_snap', {
   'bananas': [PASS, NO_VARIANTS],
   'raspberries': [FAIL, NO_VARIANTS],
 }],

tools/unittests/testdata/testroot1/v8_build_config.json

@@ -17,6 +17,7 @@
   "v8_enable_i18n_support": true,
   "v8_enable_verify_predictable": false,
   "v8_target_cpu": "x64",
+  "v8_use_snapshot": true,
   "v8_enable_embedded_builtins": false,
   "v8_enable_verify_csa": false,
   "v8_enable_lite_mode": false,

tools/unittests/testdata/testroot2/v8_build_config.json

@@ -17,6 +17,7 @@
   "v8_enable_i18n_support": true,
   "v8_enable_verify_predictable": false,
   "v8_target_cpu": "x64",
+  "v8_use_snapshot": true,
   "v8_enable_embedded_builtins": false,
   "v8_enable_verify_csa": false,
   "v8_enable_lite_mode": false,