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

This is a reland of 1c56974f

This is a plain reland of the original CL. The original CL was speculatively
reverted, but ended up not being the cause for bot failures.

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

Bug: v8:8531
Change-Id: Id75a802279238138f7aefec62e0b6425a5acc08d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864649Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Tamer Tas <tmrts@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64305}

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_use_snapshot
+  v8_enable_shared_ro_heap = !v8_enable_pointer_compression
 }
 if (v8_enable_fast_torque == "") {
   v8_enable_fast_torque = v8_enable_fast_mksnapshot
@@ -261,14 +261,10 @@ 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 " +
@@ -447,11 +443,8 @@ config("features") {
   if (v8_enable_handle_zapping) {
     defines += [ "ENABLE_HANDLE_ZAPPING" ]
   }
-  if (v8_use_snapshot) {
-    defines += [ "V8_USE_SNAPSHOT" ]
-    if (v8_enable_snapshot_native_code_counters) {
-      defines += [ "V8_SNAPSHOT_NATIVE_CODE_COUNTERS" ]
-    }
+  if (v8_enable_snapshot_native_code_counters) {
+    defines += [ "V8_SNAPSHOT_NATIVE_CODE_COUNTERS" ]
   }
   if (v8_enable_single_generation) {
     defines += [ "V8_ENABLE_SINGLE_GENERATION" ]
@@ -1335,31 +1328,29 @@ template("run_mksnapshot") {
   }
 }
 
-if (v8_use_snapshot) {
-  run_mksnapshot("default") {
+run_mksnapshot("default") {
+  args = []
+  if (v8_enable_embedded_builtins) {
+    embedded_variant = "Default"
+  }
+}
+if (emit_builtins_as_inline_asm) {
+  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 = "Default"
+      embedded_variant = "Trusted"
     }
   }
   if (emit_builtins_as_inline_asm) {
-    asm_to_inline_asm("default") {
+    asm_to_inline_asm("trusted") {
       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") {
@@ -1389,7 +1380,6 @@ 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",
@@ -1410,19 +1400,13 @@ action("v8_dump_build_config") {
 #
 
 source_set("v8_maybe_snapshot") {
-  if (v8_use_snapshot && v8_use_external_startup_data) {
+  if (v8_use_external_startup_data) {
     public_deps = [
       ":v8_external_snapshot",
     ]
-  } 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",
+      ":v8_snapshot",
     ]
   }
 }
@@ -1448,7 +1432,7 @@ v8_source_set("v8_nosnapshot") {
   configs = [ ":internal_config" ]
 }
 
-if (v8_use_snapshot && !v8_use_external_startup_data) {
+if (!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.
@@ -1490,7 +1474,7 @@ if (v8_use_snapshot && !v8_use_external_startup_data) {
   }
 }
 
-if (v8_use_snapshot && v8_use_external_startup_data) {
+if (v8_use_external_startup_data) {
   v8_source_set("v8_external_snapshot") {
     visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -3914,7 +3898,7 @@ if (current_toolchain == v8_generator_toolchain) {
   }
 }
 
-if (v8_use_snapshot && current_toolchain == v8_snapshot_toolchain) {
+if (current_toolchain == v8_snapshot_toolchain) {
   v8_executable("mksnapshot") {
     visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -4190,13 +4174,10 @@ 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" ]
@@ -4218,13 +4199,10 @@ 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,10 +35,6 @@ 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
 
@@ -66,8 +62,8 @@ declare_args() {
 
 if (v8_use_external_startup_data == "") {
   # If not specified as a gn arg, use external startup data by default if
-  # a snapshot is used and if we're not on ios.
-  v8_use_external_startup_data = v8_use_snapshot && !is_ios
+  # we're not on ios.
+  v8_use_external_startup_data = !is_ios
 }
 
 if (v8_use_multi_snapshots) {

infra/mb/mb_config.pyl

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

src/heap/heap-inl.h

@@ -209,9 +209,7 @@ 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,8 +1031,6 @@ 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)");
@@ -1058,7 +1056,6 @@ 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,10 +26,9 @@ 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(V8_USE_SNAPSHOT) && !defined(DEBUG)
+#if defined(V8_EMBEDDED_BUILTINS) && !defined(DEBUG)
 static const char* kBuild = "shipping";
 #else
 static const char* kBuild = "non-shipping";

test/unittests/logging/counters-unittest.cc

@@ -149,16 +149,11 @@ 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,7 +194,9 @@ 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']
-    self.no_snap = not build_config['v8_use_snapshot']
+    # TODO(https://crbug.com/v8/8531)
+    # 'v8_use_snapshot' was removed, 'no_snap' can be removed as well.
+    self.no_snap = False
     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_use_snapshot=False, v8_enable_embedded_builtins=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,7 +355,6 @@ 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')
@@ -572,7 +571,7 @@ class SystemTest(unittest.TestCase):
     variants.
     """
     with temp_base() as basedir:
-      override_build_config(basedir, v8_use_snapshot=False)
+      override_build_config(basedir, is_asan=True)
       result = run_tests(
           basedir,
           '--mode=Release',

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

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

tools/unittests/testdata/testroot1/v8_build_config.json

@@ -17,7 +17,6 @@
   "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,7 +17,6 @@
   "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,