[heap] Allow enabling --local-heaps by default

All tests pass now with --concurrent-allocation and --local-heaps flags
set to true.

Bug: v8:10315
Change-Id: I03a70933aa0db4d9e74933ad2fc4cb81105cb889
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2218061Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68111}

src/heap/combined-heap.cc

@@ -10,7 +10,8 @@ namespace internal {
 
 CombinedHeapObjectIterator::CombinedHeapObjectIterator(
     Heap* heap, HeapObjectIterator::HeapObjectsFiltering filtering)
-    : heap_iterator_(heap, filtering),
+    : safepoint_scope_(heap),
+      heap_iterator_(heap, filtering),
       ro_heap_iterator_(heap->isolate()->read_only_heap()) {}
 
 HeapObject CombinedHeapObjectIterator::Next() {

src/heap/combined-heap.h

@@ -7,6 +7,7 @@
 
 #include "src/heap/heap.h"
 #include "src/heap/read-only-heap.h"
+#include "src/heap/safepoint.h"
 #include "src/heap/third-party/heap-api.h"
 #include "src/objects/objects.h"
 
@@ -25,6 +26,7 @@ class V8_EXPORT_PRIVATE CombinedHeapObjectIterator final {
   HeapObject Next();
 
  private:
+  SafepointScope safepoint_scope_;
   HeapObjectIterator heap_iterator_;
   ReadOnlyHeapObjectIterator ro_heap_iterator_;
 };

src/heap/heap.cc

@@ -6,6 +6,7 @@
 
 #include <cinttypes>
 #include <iomanip>
+#include <memory>
 #include <unordered_map>
 #include <unordered_set>
 
@@ -3240,6 +3241,7 @@ FixedArrayBase Heap::LeftTrimFixedArray(FixedArrayBase object,
   if (FLAG_enable_slow_asserts) {
     // Make sure the stack or other roots (e.g., Handles) don't contain pointers
     // to the original FixedArray (which is now the filler object).
+    SafepointScope scope(this);
     LeftTrimmerVerifierRootVisitor root_visitor(object);
     ReadOnlyRoots(this).Iterate(&root_visitor);
     IterateRoots(&root_visitor, {});
@@ -4206,6 +4208,7 @@ class VerifyReadOnlyPointersVisitor : public VerifyPointersVisitor {
 
 void Heap::Verify() {
   CHECK(HasBeenSetUp());
+  SafepointScope safepoint_scope(this);
   HandleScope scope(isolate());
 
   if (FLAG_local_heaps) {
@@ -6046,6 +6049,7 @@ class UnreachableObjectsFilter : public HeapObjectsFilter {
 HeapObjectIterator::HeapObjectIterator(
     Heap* heap, HeapObjectIterator::HeapObjectsFiltering filtering)
     : heap_(heap),
+      safepoint_scope_(std::make_unique<SafepointScope>(heap)),
       filtering_(filtering),
       filter_(nullptr),
       space_iterator_(nullptr),

src/heap/heap.h

@@ -88,6 +88,7 @@ class Page;
 class PagedSpace;
 class ReadOnlyHeap;
 class RootVisitor;
+class SafepointScope;
 class ScavengeJob;
 class Scavenger;
 class ScavengerCollector;
@@ -2507,6 +2508,7 @@ class V8_EXPORT_PRIVATE HeapObjectIterator {
   DISALLOW_HEAP_ALLOCATION(no_heap_allocation_)
 
   Heap* heap_;
+  std::unique_ptr<SafepointScope> safepoint_scope_;
   HeapObjectsFiltering filtering_;
   HeapObjectsFilter* filter_;
   // Space iterator for iterating all the spaces.

src/heap/safepoint.cc

@@ -13,13 +13,16 @@ namespace v8 {
 namespace internal {
 
 GlobalSafepoint::GlobalSafepoint(Heap* heap)
-    : heap_(heap), local_heaps_head_(nullptr), is_active_(false) {}
+    : heap_(heap), local_heaps_head_(nullptr), active_safepoint_scopes_(0) {}
 
-void GlobalSafepoint::Start() { StopThreads(); }
+void GlobalSafepoint::Start() { EnterSafepointScope(); }
 
-void GlobalSafepoint::End() { ResumeThreads(); }
+void GlobalSafepoint::End() { LeaveSafepointScope(); }
 
-void GlobalSafepoint::StopThreads() {
+void GlobalSafepoint::EnterSafepointScope() {
+  if (!FLAG_local_heaps) return;
+
+  if (++active_safepoint_scopes_ > 1) return;
   local_heaps_mutex_.Lock();
 
   barrier_.Arm();
@@ -37,12 +40,13 @@ void GlobalSafepoint::StopThreads() {
       current->state_change_.Wait(&current->state_mutex_);
     }
   }
-
-  is_active_ = true;
 }
 
-void GlobalSafepoint::ResumeThreads() {
-  is_active_ = false;
+void GlobalSafepoint::LeaveSafepointScope() {
+  if (!FLAG_local_heaps) return;
+
+  DCHECK_GT(active_safepoint_scopes_, 0);
+  if (--active_safepoint_scopes_ > 0) return;
 
   for (LocalHeap* current = local_heaps_head_; current;
        current = current->next_) {
@@ -90,12 +94,10 @@ void GlobalSafepoint::Barrier::Wait() {
 }
 
 SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) {
-  if (FLAG_local_heaps) safepoint_->StopThreads();
+  safepoint_->EnterSafepointScope();
 }
 
-SafepointScope::~SafepointScope() {
-  if (FLAG_local_heaps) safepoint_->ResumeThreads();
-}
+SafepointScope::~SafepointScope() { safepoint_->LeaveSafepointScope(); }
 
 void GlobalSafepoint::AddLocalHeap(LocalHeap* local_heap) {
   base::MutexGuard guard(&local_heaps_mutex_);

src/heap/safepoint.h

@@ -47,7 +47,7 @@ class GlobalSafepoint {
   void Start();
   void End();
 
-  bool IsActive() { return is_active_; }
+  bool IsActive() { return active_safepoint_scopes_ > 0; }
 
  private:
   class Barrier {
@@ -63,8 +63,8 @@ class GlobalSafepoint {
     void Wait();
   };
 
-  void StopThreads();
-  void ResumeThreads();
+  void EnterSafepointScope();
+  void LeaveSafepointScope();
 
   void AddLocalHeap(LocalHeap* local_heap);
   void RemoveLocalHeap(LocalHeap* local_heap);
@@ -75,7 +75,7 @@ class GlobalSafepoint {
   base::Mutex local_heaps_mutex_;
   LocalHeap* local_heaps_head_;
 
-  bool is_active_;
+  int active_safepoint_scopes_;
 
   friend class SafepointScope;
   friend class LocalHeap;

src/profiler/heap-snapshot-generator.cc

@@ -11,6 +11,7 @@
 #include "src/debug/debug.h"
 #include "src/handles/global-handles.h"
 #include "src/heap/combined-heap.h"
+#include "src/heap/safepoint.h"
 #include "src/numbers/conversions.h"
 #include "src/objects/allocation-site-inl.h"
 #include "src/objects/api-callbacks.h"
@@ -2037,6 +2038,7 @@ bool HeapSnapshotGenerator::GenerateSnapshot() {
                                   GarbageCollectionReason::kHeapProfiler);
 
   NullContextForSnapshotScope null_context_scope(Isolate::FromHeap(heap_));
+  SafepointScope scope(heap_);
 
 #ifdef VERIFY_HEAP
   Heap* debug_heap = heap_;

test/cctest/heap/heap-utils.cc

@@ -10,6 +10,7 @@
 #include "src/heap/incremental-marking.h"
 #include "src/heap/mark-compact.h"
 #include "src/heap/memory-chunk.h"
+#include "src/heap/safepoint.h"
 #include "test/cctest/cctest.h"
 
 namespace v8 {
@@ -177,6 +178,7 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) {
     marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
                   i::StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
+      SafepointScope scope(heap);
       marking->FinalizeIncrementally();
     }
   }

test/cctest/heap/test-embedder-tracing.cc

@@ -8,6 +8,7 @@
 #include "include/v8.h"
 #include "src/api/api-inl.h"
 #include "src/heap/heap-inl.h"
+#include "src/heap/safepoint.h"
 #include "src/objects/module.h"
 #include "src/objects/objects-inl.h"
 #include "src/objects/script.h"
@@ -264,7 +265,11 @@ TEST(FinalizeTracingWhenMarking) {
   CHECK(i_isolate->heap()->incremental_marking()->IsStopped());
 
   i::IncrementalMarking* marking = i_isolate->heap()->incremental_marking();
+  {
+    SafepointScope scope(i_isolate->heap());
     marking->Start(i::GarbageCollectionReason::kTesting);
+  }
+
   // Sweeping is not runing so we should immediately start marking.
   CHECK(marking->IsMarking());
   tracer.FinalizeTracing();

test/cctest/heap/test-heap.cc

@@ -47,6 +47,7 @@
 #include "src/heap/memory-chunk.h"
 #include "src/heap/memory-reducer.h"
 #include "src/heap/remembered-set-inl.h"
+#include "src/heap/safepoint.h"
 #include "src/ic/ic.h"
 #include "src/numbers/hash-seed-inl.h"
 #include "src/objects/elements.h"
@@ -5616,6 +5617,7 @@ TEST(Regress598319) {
                   i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
+      SafepointScope scope(heap);
       marking->FinalizeIncrementally();
     }
   }
@@ -5699,6 +5701,7 @@ TEST(Regress615489) {
     marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
+      SafepointScope scope(heap);
       marking->FinalizeIncrementally();
     }
   }
@@ -5761,6 +5764,7 @@ TEST(Regress631969) {
     marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
+      SafepointScope scope(heap);
       marking->FinalizeIncrementally();
     }
   }
@@ -6340,6 +6344,7 @@ HEAP_TEST(Regress670675) {
   }
   i::IncrementalMarking* marking = CcTest::heap()->incremental_marking();
   if (marking->IsStopped()) {
+    SafepointScope scope(heap);
     marking->Start(i::GarbageCollectionReason::kTesting);
   }
   size_t array_length = 128 * KB;
@@ -6998,6 +7003,7 @@ TEST(Regress978156) {
   // 5. Start incremental marking.
   i::IncrementalMarking* marking = heap->incremental_marking();
   if (marking->IsStopped()) {
+    SafepointScope scope(heap);
     marking->Start(i::GarbageCollectionReason::kTesting);
   }
   IncrementalMarking::MarkingState* marking_state = marking->marking_state();

test/cctest/heap/test-incremental-marking.cc

@@ -4,6 +4,8 @@
 
 #include <stdlib.h>
 
+#include "src/heap/safepoint.h"
+
 #ifdef __linux__
 #include <errno.h>
 #include <fcntl.h>
@@ -104,7 +106,10 @@ TEST(IncrementalMarkingUsingTasks) {
   i::heap::SimulateFullSpace(CcTest::heap()->old_space());
   i::IncrementalMarking* marking = CcTest::heap()->incremental_marking();
   marking->Stop();
+  {
+    SafepointScope scope(CcTest::heap());
     marking->Start(i::GarbageCollectionReason::kTesting);
+  }
   CHECK(platform.PendingTask());
   while (platform.PendingTask()) {
     platform.PerformTask();