From ae62f7fefec3fe551735e06365ad341663ac9ca1 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Thu, 28 Mar 2019 18:00:10 +0000 Subject: [PATCH] [vm, gc] Make incremental write-barrier elimination safe. If generated code allocates an old object during concurrent marking, add this object to the deferred marking stack to be (re)scanned when marking is finalized to catch stores missed by the barrier elimination. Bug: https://github.com/dart-lang/sdk/issues/36341 Change-Id: Ifc744fdf720446f14b68268383e1fe5c92d9b5a5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97861 Reviewed-by: Siva Annamalai Reviewed-by: Vyacheslav Egorov Commit-Queue: Ryan Macnak --- .../vm/compiler/assembler/assembler_arm.cc | 5 +++ .../vm/compiler/assembler/assembler_arm64.cc | 5 +++ .../vm/compiler/assembler/assembler_ia32.cc | 5 +++ .../vm/compiler/assembler/assembler_x64.cc | 5 +++ runtime/vm/heap/marker.cc | 32 ++++++++++++++----- runtime/vm/runtime_entry.cc | 22 +++++++++++-- 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/runtime/vm/compiler/assembler/assembler_arm.cc b/runtime/vm/compiler/assembler/assembler_arm.cc index d441d61b69b..641970d09f2 100644 --- a/runtime/vm/compiler/assembler/assembler_arm.cc +++ b/runtime/vm/compiler/assembler/assembler_arm.cc @@ -1747,6 +1747,11 @@ void Assembler::StoreIntoObjectNoBarrier(Register object, #if defined(DEBUG) Label done; StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate); + + ldrb(TMP, FieldAddress(object, target::Object::tags_offset())); + tst(TMP, Operand(1 << target::RawObject::kOldAndNotRememberedBit)); + b(&done, ZERO); + Stop("Store buffer update is required"); Bind(&done); #endif // defined(DEBUG) diff --git a/runtime/vm/compiler/assembler/assembler_arm64.cc b/runtime/vm/compiler/assembler/assembler_arm64.cc index b41c7a4b195..afe4a0ce9e0 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64.cc +++ b/runtime/vm/compiler/assembler/assembler_arm64.cc @@ -1093,6 +1093,11 @@ void Assembler::StoreIntoObjectNoBarrier(Register object, #if defined(DEBUG) Label done; StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate); + + ldr(TMP, FieldAddress(object, target::Object::tags_offset()), kUnsignedByte); + tsti(TMP, Immediate(1 << target::RawObject::kOldAndNotRememberedBit)); + b(&done, ZERO); + Stop("Store buffer update is required"); Bind(&done); #endif // defined(DEBUG) diff --git a/runtime/vm/compiler/assembler/assembler_ia32.cc b/runtime/vm/compiler/assembler/assembler_ia32.cc index f1ef6b1857f..ebb652ef7f2 100644 --- a/runtime/vm/compiler/assembler/assembler_ia32.cc +++ b/runtime/vm/compiler/assembler/assembler_ia32.cc @@ -1910,6 +1910,11 @@ void Assembler::StoreIntoObjectNoBarrier(Register object, Label done; pushl(value); StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate); + + testb(FieldAddress(object, target::Object::tags_offset()), + Immediate(1 << target::RawObject::kOldAndNotRememberedBit)); + j(ZERO, &done, Assembler::kNearJump); + Stop("Store buffer update is required"); Bind(&done); popl(value); diff --git a/runtime/vm/compiler/assembler/assembler_x64.cc b/runtime/vm/compiler/assembler/assembler_x64.cc index af6b56445ba..dd8d22f2bbe 100644 --- a/runtime/vm/compiler/assembler/assembler_x64.cc +++ b/runtime/vm/compiler/assembler/assembler_x64.cc @@ -1371,6 +1371,11 @@ void Assembler::StoreIntoObjectNoBarrier(Register object, Label done; pushq(value); StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate); + + testb(FieldAddress(object, target::Object::tags_offset()), + Immediate(1 << target::RawObject::kOldAndNotRememberedBit)); + j(ZERO, &done, Assembler::kNearJump); + Stop("Store buffer update is required"); Bind(&done); popq(value); diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc index 6f4a1832c32..7ff24beae4d 100644 --- a/runtime/vm/heap/marker.cc +++ b/runtime/vm/heap/marker.cc @@ -316,17 +316,30 @@ class MarkingVisitorBase : public ObjectPointerVisitor { return raw_weak->VisitPointersNonvirtual(this); } - void FinalizeInstructions() { + void ProcessDeferredMarking() { RawObject* raw_obj; while ((raw_obj = deferred_work_list_.Pop()) != NULL) { - ASSERT(raw_obj->IsInstructions()); - RawInstructions* instr = static_cast(raw_obj); - if (TryAcquireMarkBit(instr)) { - intptr_t size = instr->HeapSize(); + ASSERT(raw_obj->IsHeapObject() && raw_obj->IsOldObject()); + // N.B. We are scanning the object even if it is already marked. + const intptr_t class_id = raw_obj->GetClassId(); + intptr_t size; + if (class_id != kWeakPropertyCid) { + size = raw_obj->VisitPointersNonvirtual(this); + } else { + RawWeakProperty* raw_weak = static_cast(raw_obj); + size = ProcessWeakProperty(raw_weak); + } + // Add the size only if we win the marking race to prevent + // double-counting. + if (TryAcquireMarkBit(raw_obj)) { marked_bytes_ += size; - NOT_IN_PRODUCT(UpdateLiveOld(kInstructionsCid, size)); + NOT_IN_PRODUCT(UpdateLiveOld(class_id, size)); } } + } + + void FinalizeDeferredMarking() { + ProcessDeferredMarking(); deferred_work_list_.Finalize(); } @@ -654,6 +667,8 @@ class ParallelMarkTask : public ThreadPool::Task { // Phase 1: Iterate over roots and drain marking stack in tasks. marker_->IterateRoots(visitor_); + visitor_->ProcessDeferredMarking(); + bool more_to_mark = false; do { do { @@ -707,7 +722,7 @@ class ParallelMarkTask : public ThreadPool::Task { barrier_->Sync(); } while (more_to_mark); - visitor_->FinalizeInstructions(); + visitor_->FinalizeDeferredMarking(); // Phase 2: Weak processing and follow-up marking on main thread. barrier_->Sync(); @@ -925,8 +940,9 @@ void GCMarker::MarkObjects(PageSpace* page_space, bool collect_code) { skipped_code_functions); ResetRootSlices(); IterateRoots(&mark); + mark.ProcessDeferredMarking(); mark.DrainMarkingStack(); - mark.FinalizeInstructions(); + mark.FinalizeDeferredMarking(); { TIMELINE_FUNCTION_GC_DURATION(thread, "ProcessWeakHandles"); MarkingWeakVisitor mark_weak(thread); diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 6df0d19542b..d4c12b16f6f 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -220,9 +220,9 @@ DEFINE_RUNTIME_ENTRY(IntegerDivisionByZeroException, 0) { } static void EnsureNewOrRemembered(Thread* thread, const Object& result) { - // For write barrier elimination, we need to ensure that the allocation ends - // up in the new space if Heap::IsGuaranteedNewSpaceAllocation is true for - // this size or else the object needs to go into the store buffer. + // For generational write barrier elimination, we need to ensure that the + // allocation ends up in the new space if Heap::IsGuaranteedNewSpaceAllocation + // is true for this size or else the object needs to go into the store buffer. NoSafepointScope no_safepoint_scope; RawObject* object = result.raw(); @@ -231,6 +231,18 @@ static void EnsureNewOrRemembered(Thread* thread, const Object& result) { } } +static void EnsureNewOrDeferredMarking(Thread* thread, const Object& result) { + // For incremental write barrier elimination, we need to ensure that the + // allocation ends up in the new space or else the object needs to added + // to deferred marking stack so it will be [re]scanned. + NoSafepointScope no_safepoint_scope; + + RawObject* object = result.raw(); + if (object->IsOldObject() && thread->is_marking()) { + thread->DeferredMarkingStackAddObject(object); + } +} + // Allocation of a fixed length array of given element type. // This runtime entry is never called for allocating a List of a generic type, // because a prior run time call instantiates the element type if necessary. @@ -266,6 +278,8 @@ DEFINE_RUNTIME_ENTRY(AllocateArray, 2) { if (!array.raw()->IsCardRemembered()) { EnsureNewOrRemembered(thread, array); } + EnsureNewOrDeferredMarking(thread, array); + return; } } @@ -314,6 +328,7 @@ DEFINE_RUNTIME_ENTRY(AllocateObject, 2) { if (AllocateObjectInstr::WillAllocateNewOrRemembered(cls)) { EnsureNewOrRemembered(thread, instance); + EnsureNewOrDeferredMarking(thread, instance); } } @@ -425,6 +440,7 @@ DEFINE_RUNTIME_ENTRY(AllocateContext, 1) { AllocateUninitializedContextInstr::WillAllocateNewOrRemembered( num_context_variables)) { EnsureNewOrRemembered(thread, context); + EnsureNewOrDeferredMarking(thread, context); } }