[heap] Only start incremental marking when V8 is not in GC VM state.

Bug: v8:12503
Change-Id: Icda291d9770c46c7fee3c70dd4df97f320b1956a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3398113Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79623}

src/execution/isolate.h

@@ -1326,9 +1326,15 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   THREAD_LOCAL_TOP_ACCESSOR(ExternalCallbackScope*, external_callback_scope)
 
-  THREAD_LOCAL_TOP_ACCESSOR(StateTag, current_vm_state)
   THREAD_LOCAL_TOP_ACCESSOR(EmbedderState*, current_embedder_state)
 
+  inline void set_current_vm_state(StateTag state_tag) {
+    current_vm_state_.store(state_tag, std::memory_order_relaxed);
+  }
+  inline StateTag current_vm_state() const {
+    return current_vm_state_.load(std::memory_order_relaxed);
+  }
+
   void SetData(uint32_t slot, void* data) {
     DCHECK_LT(slot, Internals::kNumIsolateDataSlots);
     isolate_data_.embedder_data_[slot] = data;
@@ -2072,6 +2078,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   ReadOnlyHeap* read_only_heap_ = nullptr;
   std::shared_ptr<ReadOnlyArtifacts> artifacts_;
   std::shared_ptr<StringTable> string_table_;
+  std::atomic<StateTag> current_vm_state_{EXTERNAL};
 
   const int id_;
   EntryStackItem* entry_stack_ = nullptr;

src/execution/thread-local-top.cc

@@ -30,7 +30,6 @@ void ThreadLocalTop::Clear() {
   simulator_ = nullptr;
   js_entry_sp_ = kNullAddress;
   external_callback_scope_ = nullptr;
-  current_vm_state_ = EXTERNAL;
   current_embedder_state_ = nullptr;
   failed_access_check_callback_ = nullptr;
   thread_in_wasm_flag_address_ = kNullAddress;

src/execution/thread-local-top.h

@@ -34,9 +34,9 @@ class ThreadLocalTop {
   // refactor this to really consist of just Addresses and 32-bit
   // integer fields.
 #ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
-  static constexpr uint32_t kSizeInBytes = 26 * kSystemPointerSize;
-#else
   static constexpr uint32_t kSizeInBytes = 25 * kSystemPointerSize;
+#else
+  static constexpr uint32_t kSizeInBytes = 24 * kSystemPointerSize;
 #endif
 
   // Does early low-level initialization that does not depend on the
@@ -146,7 +146,6 @@ class ThreadLocalTop {
   Address js_entry_sp_;
   // The external callback we're currently in.
   ExternalCallbackScope* external_callback_scope_;
-  StateTag current_vm_state_;
   EmbedderState* current_embedder_state_;
 
   // Call back function to report unsafe JS accesses.

src/heap/heap.cc

@@ -1698,11 +1698,10 @@ void Heap::ReportExternalMemoryPressure() {
     return;
   }
   if (incremental_marking()->IsStopped()) {
-    if (incremental_marking()->CanBeActivated()) {
-      StartIncrementalMarking(GCFlagsForIncrementalMarking(),
+    if (!StartIncrementalMarking(
+            GCFlagsForIncrementalMarking(),
             GarbageCollectionReason::kExternalMemoryPressure,
-                              kGCCallbackFlagsForExternalMemory);
-    } else {
+            kGCCallbackFlagsForExternalMemory)) {
       CollectAllGarbage(i::Heap::kNoGCFlags,
                         GarbageCollectionReason::kExternalMemoryPressure,
                         kGCCallbackFlagsForExternalMemory);
@@ -1986,10 +1985,11 @@ int Heap::NotifyContextDisposed(bool dependant_context) {
   return ++contexts_disposed_;
 }
 
-void Heap::StartIncrementalMarking(int gc_flags,
+bool Heap::StartIncrementalMarking(int gc_flags,
                                    GarbageCollectionReason gc_reason,
                                    GCCallbackFlags gc_callback_flags) {
   DCHECK(incremental_marking()->IsStopped());
+  if (!incremental_marking()->CanBeActivated()) return false;
 
   // Sweeping needs to be completed such that markbits are all cleared before
   // starting marking again.
@@ -2014,6 +2014,7 @@ void Heap::StartIncrementalMarking(int gc_flags,
   set_current_gc_flags(gc_flags);
   current_gc_callback_flags_ = gc_callback_flags;
   incremental_marking()->Start(gc_reason);
+  return true;
 }
 
 void Heap::CompleteSweepingFull() {
@@ -4168,7 +4169,7 @@ void Heap::CheckMemoryPressure() {
     TRACE_EVENT0("devtools.timeline,v8", "V8.CheckMemoryPressure");
     CollectGarbageOnMemoryPressure();
   } else if (memory_pressure_level == MemoryPressureLevel::kModerate) {
-    if (FLAG_incremental_marking && incremental_marking()->IsStopped()) {
+    if (incremental_marking()->IsStopped()) {
       TRACE_EVENT0("devtools.timeline,v8", "V8.CheckMemoryPressure");
       StartIncrementalMarking(kReduceMemoryFootprintMask,
                               GarbageCollectionReason::kMemoryPressure);

src/heap/heap.h

@@ -1064,7 +1064,9 @@ class Heap {
 
   // Starts incremental marking assuming incremental marking is currently
   // stopped.
-  V8_EXPORT_PRIVATE void StartIncrementalMarking(
+  // Returns true if starting was successful. Returns false if marking cannot be
+  // activated.
+  V8_EXPORT_PRIVATE bool StartIncrementalMarking(
       int gc_flags, GarbageCollectionReason gc_reason,
       GCCallbackFlags gc_callback_flags = GCCallbackFlags::kNoGCCallbackFlags);
 
@@ -1309,7 +1311,7 @@ class Heap {
   size_t CommittedMemoryOfUnmapper();
 
   // Returns the amount of memory currently committed for the heap.
-  size_t CommittedMemory();
+  V8_EXPORT_PRIVATE size_t CommittedMemory();
 
   // Returns the amount of memory currently committed for the old space.
   size_t CommittedOldGenerationMemory();

src/heap/incremental-marking.cc

@@ -146,7 +146,8 @@ bool IncrementalMarking::CanBeActivated() {
   //   2) when we are currently not in a GC, and
   //   3) when we are currently not serializing or deserializing the heap, and
   //   4) not a shared heap.
-  return FLAG_incremental_marking && heap_->gc_state() == Heap::NOT_IN_GC &&
+  return FLAG_incremental_marking &&
+         heap_->isolate()->current_vm_state() != GC &&
          heap_->deserialization_complete() &&
          !heap_->isolate()->serializer_enabled() && !heap_->IsShared();
 }

test/cctest/heap/test-heap.cc

@@ -6506,7 +6506,7 @@ void OOMCallback(const char* location, bool is_heap_oom) {
   Heap* heap = oom_isolate->heap();
   size_t kSlack = heap->new_space() ? heap->new_space()->Capacity() : 0;
   CHECK_LE(heap->OldGenerationCapacity(), kHeapLimit + kSlack);
-  CHECK_LE(heap->memory_allocator()->Size(), heap->MaxReserved() + kSlack);
+  CHECK_LE(heap->memory_allocator()->Size(), heap->CommittedMemory());
   base::OS::ExitProcess(0);
 }