Skip to content

Commit

Permalink
[heap] Add concurrent typed slot recording
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
ulan authored and Commit Bot committed Aug 26, 2020
1 parent db1115e commit 9eb090d
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 17 deletions.
31 changes: 28 additions & 3 deletions src/heap/marking-barrier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -146,6 +170,7 @@ void MarkingBarrier::Deactivate() {
p->SetOldGenerationPageFlags(false);
}
}
DCHECK(typed_slots_map_.empty());
DCHECK(worklist_.IsLocalEmpty());
}

Expand Down
5 changes: 5 additions & 0 deletions src/heap/marking-barrier.h
Original file line number Diff line number Diff line change
Expand Up @@ -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*);

Expand All @@ -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_;
Expand Down
5 changes: 3 additions & 2 deletions test/cctest/cctest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion test/cctest/cctest.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions test/cctest/heap/heap-utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
CcTest::CollectAllGarbage();
Expand Down
4 changes: 2 additions & 2 deletions test/cctest/heap/heap-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
81 changes: 80 additions & 1 deletion test/cctest/heap/test-concurrent-allocation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}
Expand All @@ -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();
}
Expand Down
12 changes: 6 additions & 6 deletions test/cctest/heap/test-embedder-tracing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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) {
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down

0 comments on commit 9eb090d

Please sign in to comment.