Reland "[heap] Add concurrent typed slot recording"

This is a reland of 9eb090d261

The android-pie-arm64-dbg compiler error was fixed in:
https://chromium-review.googlesource.com/c/v8/v8/+/2381450

Original change's description:
> [heap] Add concurrent typed slot recording
>
> Since the typed slot set is not thread-safe, each concurrent marking
> barrier collects typed slots locally and publishes them to the main
> typed slot set in safepoints.
> Bug: v8:10315
>
> Change-Id: If1f5c5df786df88aac7bc27088afe91a4173c826
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2370302
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69576}

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

src/heap/marking-barrier.cc

@@ -45,17 +45,21 @@ void MarkingBarrier::Write(HeapObject host, HeapObjectSlot slot,
 }
 
 void MarkingBarrier::Write(Code host, RelocInfo* reloc_info, HeapObject value) {
-  DCHECK(is_main_thread_barrier_);
   if (MarkValue(host, value)) {
     if (is_compacting_) {
-      collector_->RecordRelocSlot(host, reloc_info, value);
+      if (is_main_thread_barrier_) {
+        // An optimization to avoid allocating additional typed slots for the
+        // main thread.
+        collector_->RecordRelocSlot(host, reloc_info, value);
+      } else {
+        RecordRelocSlot(host, reloc_info, value);
+      }
     }
   }
 }
 
 void MarkingBarrier::Write(JSArrayBuffer host,
                            ArrayBufferExtension* extension) {
-  DCHECK(is_main_thread_barrier_);
   if (!V8_CONCURRENT_MARKING_BOOL && marking_state_.IsBlack(host)) {
     // The extension will be marked when the marker visits the host object.
     return;
@@ -74,6 +78,19 @@ void MarkingBarrier::Write(Map host, DescriptorArray descriptor_array,
   }
 }
 
+void MarkingBarrier::RecordRelocSlot(Code host, RelocInfo* rinfo,
+                                     HeapObject target) {
+  MarkCompactCollector::RecordRelocSlotInfo info =
+      MarkCompactCollector::PrepareRecordRelocSlot(host, rinfo, target);
+  if (info.should_record) {
+    auto& typed_slots = typed_slots_map_[info.memory_chunk];
+    if (!typed_slots) {
+      typed_slots.reset(new TypedSlots());
+    }
+    typed_slots->Insert(info.slot_type, info.offset);
+  }
+}
+
 // static
 void MarkingBarrier::ActivateAll(Heap* heap, bool is_compacting) {
   heap->marking_barrier()->Activate(is_compacting);
@@ -109,6 +126,13 @@ void MarkingBarrier::Publish() {
   DCHECK_IMPLIES(!is_main_thread_barrier_, FLAG_local_heaps);
   if (is_activated_) {
     worklist_.Publish();
+    for (auto& it : typed_slots_map_) {
+      MemoryChunk* memory_chunk = it.first;
+      std::unique_ptr<TypedSlots>& typed_slots = it.second;
+      RememberedSet<OLD_TO_OLD>::MergeTyped(memory_chunk,
+                                            std::move(typed_slots));
+    }
+    typed_slots_map_.clear();
   }
 }
 
@@ -146,6 +170,7 @@ void MarkingBarrier::Deactivate() {
       p->SetOldGenerationPageFlags(false);
     }
   }
+  DCHECK(typed_slots_map_.empty());
   DCHECK(worklist_.IsLocalEmpty());
 }
 

src/heap/marking-barrier.h

@@ -45,6 +45,8 @@ class MarkingBarrier {
 
   inline bool WhiteToGreyAndPush(HeapObject value);
 
+  void RecordRelocSlot(Code host, RelocInfo* rinfo, HeapObject target);
+
   void ActivateSpace(PagedSpace*);
   void ActivateSpace(NewSpace*);
 
@@ -56,6 +58,9 @@ class MarkingBarrier {
   IncrementalMarking* incremental_marking_;
   MarkingWorklist::Local worklist_;
   MarkingState marking_state_;
+  std::unordered_map<MemoryChunk*, std::unique_ptr<TypedSlots>,
+                     MemoryChunk::Hasher>
+      typed_slots_map_;
   bool is_compacting_ = false;
   bool is_activated_ = false;
   bool is_main_thread_barrier_;

test/cctest/cctest.cc

@@ -129,8 +129,9 @@ i::ReadOnlyHeap* CcTest::read_only_heap() {
   return i_isolate()->read_only_heap();
 }
 
-void CcTest::CollectGarbage(i::AllocationSpace space) {
-  heap()->CollectGarbage(space, i::GarbageCollectionReason::kTesting);
+void CcTest::CollectGarbage(i::AllocationSpace space, i::Isolate* isolate) {
+  i::Isolate* iso = isolate ? isolate : i_isolate();
+  iso->heap()->CollectGarbage(space, i::GarbageCollectionReason::kTesting);
 }
 
 void CcTest::CollectAllGarbage(i::Isolate* isolate) {

test/cctest/cctest.h

@@ -137,7 +137,8 @@ class CcTest {
   static i::Heap* heap();
   static i::ReadOnlyHeap* read_only_heap();
 
-  static void CollectGarbage(i::AllocationSpace space);
+  static void CollectGarbage(i::AllocationSpace space,
+                             i::Isolate* isolate = nullptr);
   static void CollectAllGarbage(i::Isolate* isolate = nullptr);
   static void CollectAllAvailableGarbage(i::Isolate* isolate = nullptr);
   static void PreciseCollectAllGarbage(i::Isolate* isolate = nullptr);

test/cctest/heap/heap-utils.cc

@@ -18,9 +18,11 @@ namespace v8 {
 namespace internal {
 namespace heap {
 
-void InvokeScavenge() { CcTest::CollectGarbage(i::NEW_SPACE); }
+void InvokeScavenge(Isolate* isolate) {
+  CcTest::CollectGarbage(i::NEW_SPACE, isolate);
+}
 
-void InvokeMarkSweep() { CcTest::CollectAllGarbage(); }
+void InvokeMarkSweep(Isolate* isolate) { CcTest::CollectAllGarbage(isolate); }
 
 void SealCurrentObjects(Heap* heap) {
   // If you see this check failing, disable the flag at the start of your test:

test/cctest/heap/heap-utils.h

@@ -66,9 +66,9 @@ void GcAndSweep(Heap* heap, AllocationSpace space);
 
 void ForceEvacuationCandidate(Page* page);
 
-void InvokeScavenge();
+void InvokeScavenge(Isolate* isolate = nullptr);
 
-void InvokeMarkSweep();
+void InvokeMarkSweep(Isolate* isolate = nullptr);
 
 template <typename GlobalOrPersistent>
 bool InYoungGeneration(v8::Isolate* isolate, const GlobalOrPersistent& global) {

test/cctest/heap/test-concurrent-allocation.cc

@@ -8,6 +8,10 @@
 #include "src/base/platform/condition-variable.h"
 #include "src/base/platform/mutex.h"
 #include "src/base/platform/semaphore.h"
+#include "src/codegen/assembler-inl.h"
+#include "src/codegen/assembler.h"
+#include "src/codegen/macro-assembler-inl.h"
+#include "src/codegen/macro-assembler.h"
 #include "src/common/globals.h"
 #include "src/handles/handles-inl.h"
 #include "src/handles/local-handles-inl.h"
@@ -286,7 +290,8 @@ UNINITIALIZED_TEST(ConcurrentWriteBarrier) {
     HandleScope handle_scope(i_isolate);
     Handle<FixedArray> fixed_array_handle(
         i_isolate->factory()->NewFixedArray(1));
-    Handle<HeapNumber> value_handle(i_isolate->factory()->NewHeapNumber(1.1));
+    Handle<HeapNumber> value_handle(
+        i_isolate->factory()->NewHeapNumber<AllocationType::kOld>(1.1));
     fixed_array = *fixed_array_handle;
     value = *value_handle;
   }
@@ -301,6 +306,80 @@ UNINITIALIZED_TEST(ConcurrentWriteBarrier) {
   thread->Join();
 
   CHECK(heap->incremental_marking()->marking_state()->IsBlackOrGrey(value));
+  heap::InvokeMarkSweep(i_isolate);
+
+  isolate->Dispose();
+}
+
+class ConcurrentRecordRelocSlotThread final : public v8::base::Thread {
+ public:
+  explicit ConcurrentRecordRelocSlotThread(Heap* heap, Code code,
+                                           HeapObject value)
+      : v8::base::Thread(base::Thread::Options("ThreadWithLocalHeap")),
+        heap_(heap),
+        code_(code),
+        value_(value) {}
+
+  void Run() override {
+    LocalHeap local_heap(heap_);
+    int mode_mask = RelocInfo::EmbeddedObjectModeMask();
+    for (RelocIterator it(code_, mode_mask); !it.done(); it.next()) {
+      DCHECK(RelocInfo::IsEmbeddedObjectMode(it.rinfo()->rmode()));
+      it.rinfo()->set_target_object(heap_, value_);
+    }
+  }
+
+  Heap* heap_;
+  Code code_;
+  HeapObject value_;
+};
+
+UNINITIALIZED_TEST(ConcurrentRecordRelocSlot) {
+  FLAG_manual_evacuation_candidates_selection = true;
+  ManualGCScope manual_gc_scope;
+  FLAG_concurrent_allocation = true;
+  FLAG_local_heaps = true;
+
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+  Isolate* i_isolate = reinterpret_cast<Isolate*>(isolate);
+  Heap* heap = i_isolate->heap();
+
+  Code code;
+  HeapObject value;
+  {
+    HandleScope handle_scope(i_isolate);
+    i::byte buffer[i::Assembler::kDefaultBufferSize];
+    MacroAssembler masm(i_isolate, v8::internal::CodeObjectRequired::kYes,
+                        ExternalAssemblerBuffer(buffer, sizeof(buffer)));
+    masm.Push(ReadOnlyRoots(heap).undefined_value_handle());
+    CodeDesc desc;
+    masm.GetCode(i_isolate, &desc);
+    Handle<Code> code_handle =
+        Factory::CodeBuilder(i_isolate, desc, CodeKind::STUB).Build();
+    heap::AbandonCurrentlyFreeMemory(heap->old_space());
+    Handle<HeapNumber> value_handle(
+        i_isolate->factory()->NewHeapNumber<AllocationType::kOld>(1.1));
+    heap::ForceEvacuationCandidate(Page::FromHeapObject(*value_handle));
+    code = *code_handle;
+    value = *value_handle;
+  }
+  heap->StartIncrementalMarking(i::Heap::kNoGCFlags,
+                                i::GarbageCollectionReason::kTesting);
+  CHECK(heap->incremental_marking()->marking_state()->IsWhite(value));
+
+  {
+    CodeSpaceMemoryModificationScope modification_scope(heap);
+    auto thread =
+        std::make_unique<ConcurrentRecordRelocSlotThread>(heap, code, value);
+    CHECK(thread->Start());
+
+    thread->Join();
+  }
+
+  CHECK(heap->incremental_marking()->marking_state()->IsBlackOrGrey(value));
+  heap::InvokeMarkSweep(i_isolate);
 
   isolate->Dispose();
 }

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

@@ -455,7 +455,7 @@ TEST(TracedGlobalToUnmodifiedJSObjectDiesOnMarkSweep) {
   CcTest::InitializeVM();
   TracedGlobalTest(
       CcTest::isolate(), ConstructJSObject,
-      [](const TracedGlobal<v8::Object>& global) {}, InvokeMarkSweep,
+      [](const TracedGlobal<v8::Object>& global) {}, [] { InvokeMarkSweep(); },
       SurvivalMode::kDies);
 }
 
@@ -469,7 +469,7 @@ TEST(TracedGlobalToUnmodifiedJSObjectSurvivesMarkSweepWhenHeldAliveOtherwise) {
         v8::HandleScope scope(isolate);
         strong_global = v8::Global<v8::Object>(isolate, global.Get(isolate));
       },
-      InvokeMarkSweep, SurvivalMode::kSurvives);
+      []() { InvokeMarkSweep(); }, SurvivalMode::kSurvives);
 }
 
 TEST(TracedGlobalToUnmodifiedJSObjectSurvivesScavenge) {
@@ -478,7 +478,7 @@ TEST(TracedGlobalToUnmodifiedJSObjectSurvivesScavenge) {
   CcTest::InitializeVM();
   TracedGlobalTest(
       CcTest::isolate(), ConstructJSObject,
-      [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
+      [](const TracedGlobal<v8::Object>& global) {}, []() { InvokeScavenge(); },
       SurvivalMode::kSurvives);
 }
 
@@ -492,7 +492,7 @@ TEST(TracedGlobalToUnmodifiedJSObjectSurvivesScavengeWhenExcludedFromRoots) {
   tracer.ConsiderTracedGlobalAsRoot(false);
   TracedGlobalTest(
       CcTest::isolate(), ConstructJSObject,
-      [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
+      [](const TracedGlobal<v8::Object>& global) {}, []() { InvokeScavenge(); },
       SurvivalMode::kSurvives);
 }
 
@@ -506,7 +506,7 @@ TEST(TracedGlobalToUnmodifiedJSApiObjectSurvivesScavengePerDefault) {
   tracer.ConsiderTracedGlobalAsRoot(true);
   TracedGlobalTest(
       CcTest::isolate(), ConstructJSApiObject<TracedGlobal<v8::Object>>,
-      [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
+      [](const TracedGlobal<v8::Object>& global) {}, []() { InvokeScavenge(); },
       SurvivalMode::kSurvives);
 }
 
@@ -520,7 +520,7 @@ TEST(TracedGlobalToUnmodifiedJSApiObjectDiesOnScavengeWhenExcludedFromRoots) {
   tracer.ConsiderTracedGlobalAsRoot(false);
   TracedGlobalTest(
       CcTest::isolate(), ConstructJSApiObject<TracedGlobal<v8::Object>>,
-      [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
+      [](const TracedGlobal<v8::Object>& global) {}, []() { InvokeScavenge(); },
       SurvivalMode::kDies);
 }