Reland "[heap] Avoid dynamic updates of FLAG_gc_interval"

This is a reland of commit abcb6bb8b4.
The data race is fixed by using atomic operations.

Original change's description:
> [heap] Avoid dynamic updates of FLAG_gc_interval
>
> Flags will be protected from updates after V8 initialization (in the
> future). This CL avoids any updates of the --gc-interval flag during
> runtime, and instead updates a static field on the HeapAllocator
> directly.
>
> R=mlippautz@chromium.org
>
> Bug: v8:12887
> Change-Id: I17a495cae50a46d59a8159c6ece1558d4d61b949
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687691
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#80998}

Bug: v8:12887
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Change-Id: Ib5b537500413a627d9b2509354d20906e0474d8e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695380
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81008}

src/flags/flag-definitions.h

@@ -1199,10 +1199,16 @@ DEFINE_BOOL(separate_gc_phases, false,
 DEFINE_BOOL(global_gc_scheduling, true,
             "enable GC scheduling based on global memory")
 DEFINE_BOOL(gc_global, false, "always perform global GCs")
+
+// TODO(12950): The next two flags only have an effect if
+// V8_ENABLE_ALLOCATION_TIMEOUT is set, so we should only define them in that
+// config. That currently breaks Node's parallel/test-domain-error-types test
+// though.
 DEFINE_INT(random_gc_interval, 0,
            "Collect garbage after random(0, X) allocations. It overrides "
            "gc_interval.")
 DEFINE_INT(gc_interval, -1, "garbage collect after <n> allocations")
+
 DEFINE_INT(retain_maps_for_n_gc, 2,
            "keeps maps alive for <n> old space garbage collections")
 DEFINE_BOOL(trace_gc, false,

src/heap/heap-allocator.cc

@@ -141,6 +141,19 @@ void HeapAllocator::IncrementObjectCounters() {
 #endif  // DEBUG
 
 #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
+// static
+void HeapAllocator::InitializeOncePerProcess() {
+  SetAllocationGcInterval(FLAG_gc_interval);
+}
+
+// static
+void HeapAllocator::SetAllocationGcInterval(int allocation_gc_interval) {
+  allocation_gc_interval_.store(allocation_gc_interval,
+                                std::memory_order_relaxed);
+}
+
+// static
+std::atomic<int> HeapAllocator::allocation_gc_interval_{-1};
 
 void HeapAllocator::SetAllocationTimeout(int allocation_timeout) {
   allocation_timeout_ = allocation_timeout;
@@ -158,8 +171,12 @@ void HeapAllocator::UpdateAllocationTimeout() {
     // allow the subsequent allocation attempts to go through.
     constexpr int kFewAllocationsHeadroom = 6;
     allocation_timeout_ = std::max(kFewAllocationsHeadroom, new_timeout);
-  } else if (FLAG_gc_interval >= 0) {
-    allocation_timeout_ = FLAG_gc_interval;
+    return;
+  }
+
+  int interval = allocation_gc_interval_.load(std::memory_order_relaxed);
+  if (interval >= 0) {
+    allocation_timeout_ = interval;
   }
 }
 

src/heap/heap-allocator.h

@@ -70,6 +70,9 @@ class V8_EXPORT_PRIVATE HeapAllocator final {
 #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
   void UpdateAllocationTimeout();
   void SetAllocationTimeout(int allocation_timeout);
+
+  static void SetAllocationGcInterval(int allocation_gc_interval);
+  static void InitializeOncePerProcess();
 #endif  // V8_ENABLE_ALLOCATION_TIMEOUT
 
  private:
@@ -107,10 +110,15 @@ class V8_EXPORT_PRIVATE HeapAllocator final {
   ConcurrentAllocator* shared_map_allocator_;
 
 #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
-  // If the --gc-interval flag is set to a positive value, this variable
+  // If the {allocation_gc_interval_} is set to a positive value, this variable
   // holds the value indicating the number of allocations remain until the
   // next failure and garbage collection.
   int allocation_timeout_ = 0;
+
+  // The configured GC interval, initialized from --gc-interval during
+  // {InitializeOncePerProcess} and potentially dynamically updated by
+  // %SetAllocationTimeout.
+  static std::atomic<int> allocation_gc_interval_;
 #endif  // V8_ENABLE_ALLOCATION_TIMEOUT
 };
 

src/heap/heap.cc

@@ -5927,6 +5927,9 @@ void Heap::InitializeHashSeed() {
 
 // static
 void Heap::InitializeOncePerProcess() {
+#ifdef V8_ENABLE_ALLOCATION_TIMEOUT
+  HeapAllocator::InitializeOncePerProcess();
+#endif
   MemoryAllocator::InitializeOncePerProcess();
 }
 

src/runtime/runtime-test.cc

@@ -884,12 +884,12 @@ RUNTIME_FUNCTION(Runtime_SetAllocationTimeout) {
   SealHandleScope shs(isolate);
   DCHECK(args.length() == 2 || args.length() == 3);
 #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
+  CONVERT_INT32_ARG_FUZZ_SAFE(interval, 0);
+  HeapAllocator::SetAllocationGcInterval(interval);
   CONVERT_INT32_ARG_FUZZ_SAFE(timeout, 1);
   isolate->heap()->set_allocation_timeout(timeout);
 #endif
 #ifdef DEBUG
-  CONVERT_INT32_ARG_FUZZ_SAFE(interval, 0);
-  FLAG_gc_interval = interval;
   if (args.length() == 3) {
     // Enable/disable inline allocation if requested.
     CONVERT_BOOLEAN_ARG_FUZZ_SAFE(inline_allocation, 2);

test/cctest/heap/test-heap.cc

@@ -3023,7 +3023,7 @@ static void AddPropertyTo(int gc_count, Handle<JSObject> object,
   Factory* factory = isolate->factory();
   Handle<String> prop_name = factory->InternalizeUtf8String(property_name);
   Handle<Smi> twenty_three(Smi::FromInt(23), isolate);
-  FLAG_gc_interval = gc_count;
+  HeapAllocator::SetAllocationGcInterval(gc_count);
   FLAG_gc_global = true;
   FLAG_retain_maps_for_n_gc = 0;
   CcTest::heap()->set_allocation_timeout(gc_count);
@@ -4825,7 +4825,7 @@ TEST(AddInstructionChangesNewSpacePromotion) {
   FLAG_allow_natives_syntax = true;
   FLAG_expose_gc = true;
   FLAG_stress_compaction = true;
-  FLAG_gc_interval = 1000;
+  HeapAllocator::SetAllocationGcInterval(1000);
   CcTest::InitializeVM();
   if (!FLAG_allocation_site_pretenuring) return;
   v8::HandleScope scope(CcTest::isolate());
@@ -5210,8 +5210,6 @@ TEST(RetainedMapsCleanup) {
 }
 
 TEST(PreprocessStackTrace) {
-  // Do not automatically trigger early GC.
-  FLAG_gc_interval = -1;
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());
   v8::TryCatch try_catch(CcTest::isolate());
@@ -5241,7 +5239,6 @@ TEST(PreprocessStackTrace) {
   }
 }
 
-
 void AllocateInSpace(Isolate* isolate, size_t bytes, AllocationSpace space) {
   CHECK_LE(FixedArray::kHeaderSize, bytes);
   CHECK(IsAligned(bytes, kTaggedSize));

test/cctest/test-api.cc

@@ -24152,9 +24152,11 @@ TEST(CreateSyntheticModule) {
 }
 
 TEST(CreateSyntheticModuleGC) {
+#ifdef V8_ENABLE_ALLOCATION_TIMEOUT
   // Try to make sure that CreateSyntheticModule() deals well with a GC
   // happening during its execution.
-  i::FLAG_gc_interval = 10;
+  i::HeapAllocator::SetAllocationGcInterval(10);
+#endif
   i::FLAG_inline_new = false;
 
   LocalContext env;