[vm, gc] Don't rebuild the remembered set during marking.
Instead, only filter dead objects from the remembered set at the end of marking.
Allows scavenges to occur during concurrent marking, and avoids changing the mutator to handle a race with the concurrent marker for the remembered bit.
Bug: https://github.com/dart-lang/sdk/issues/34002
Change-Id: I116d7acc8fb57b8c3088febe6c196030a40fb319
Reviewed-on: https://dart-review.googlesource.com/67841
Reviewed-by: Siva Annamalai <asiva@google.com>
diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc
index 48d4bae..c352abd 100644
--- a/runtime/vm/heap/marker.cc
+++ b/runtime/vm/heap/marker.cc
@@ -152,7 +152,6 @@
page_space_(page_space),
work_list_(marking_stack),
delayed_weak_properties_(NULL),
- visiting_old_object_(NULL),
skipped_code_functions_(skipped_code_functions),
marked_bytes_(0) {
ASSERT(thread_->isolate() == isolate);
@@ -191,7 +190,6 @@
// The key is marked so we make sure to properly visit all pointers
// originating from this weak property.
- VisitingOldObject(cur_weak);
cur_weak->VisitPointersNonvirtual(this);
} else {
// Requeue this weak property to be handled later.
@@ -200,7 +198,6 @@
// Advance to next weak property in the queue.
cur_weak = reinterpret_cast<RawWeakProperty*>(next_weak);
}
- VisitingOldObject(NULL);
return marked;
}
@@ -211,13 +208,12 @@
}
if (raw_obj == NULL) {
- ASSERT(visiting_old_object_ == NULL);
return;
}
+
do {
do {
// First drain the marking stacks.
- VisitingOldObject(raw_obj);
const intptr_t class_id = raw_obj->GetClassId();
intptr_t size;
@@ -241,12 +237,11 @@
// by the handling of weak properties.
raw_obj = work_list_.Pop();
} while (raw_obj != NULL);
- VisitingOldObject(NULL);
}
void VisitPointers(RawObject** first, RawObject** last) {
for (RawObject** current = first; current <= last; current++) {
- MarkObject(*current, current);
+ MarkObject(*current);
}
}
@@ -302,11 +297,6 @@
}
}
- void VisitingOldObject(RawObject* obj) {
- ASSERT((obj == NULL) || obj->IsOldObject());
- visiting_old_object_ = obj;
- }
-
private:
void PushMarked(RawObject* raw_obj) {
ASSERT(raw_obj->IsHeapObject());
@@ -316,41 +306,26 @@
// Push the marked object on the marking stack.
ASSERT(raw_obj->IsMarked());
- // We acquired the mark bit => no other task is modifying the header.
- // TODO(koda): For concurrent mutator, this needs synchronization. Consider
- // clearing these bits already in the CAS for the mark bit.
- raw_obj->ClearRememberedBitUnsynchronized();
work_list_.Push(raw_obj);
}
static bool TryAcquireMarkBit(RawObject* raw_obj) {
+ // While it might seem this is redundant with TryAcquireMarkBit, we must
+ // do this check first to avoid attempting an AMO on the read-only
+ // vm-isolate or image pages.
+ if (raw_obj->IsMarked()) return false;
+
if (!sync) {
- if (raw_obj->IsMarked()) return false;
raw_obj->SetMarkBitUnsynchronized();
return true;
+ } else {
+ return raw_obj->TryAcquireMarkBit();
}
- return raw_obj->TryAcquireMarkBit();
}
- void MarkObject(RawObject* raw_obj, RawObject** p) {
+ void MarkObject(RawObject* raw_obj) {
// Fast exit if the raw object is a Smi.
- if (!raw_obj->IsHeapObject()) {
- return;
- }
-
- // Fast exit if the raw object is marked.
- if (raw_obj->IsMarked()) {
- return;
- }
-
- // TODO(koda): Investigate performance impact of alternative branching:
- // if (smi or new) <-- can be done as single compare + conditional jump
- // if (smi) return;
- // else ...
- // if (marked) return;
- // ...
- if (raw_obj->IsNewObject()) {
- ProcessNewSpaceObject(raw_obj, p);
+ if (raw_obj->IsSmiOrNewObject()) {
return;
}
@@ -362,31 +337,6 @@
PushMarked(raw_obj);
}
- static bool TryAcquireRememberedBit(RawObject* raw_obj) {
- if (!sync) {
- if (raw_obj->IsRemembered()) return false;
- raw_obj->SetRememberedBitUnsynchronized();
- return true;
- }
- return raw_obj->TryAcquireRememberedBit();
- }
-
- void ProcessNewSpaceObject(RawObject* raw_obj, RawObject** p) {
- // TODO(iposva): Add consistency check.
- if ((visiting_old_object_ != NULL) &&
- TryAcquireRememberedBit(visiting_old_object_)) {
- // NOTE: We pass in the pointer to the address we are visiting
- // allows us to get a distance from the object start. At some
- // point we might want to store exact addresses in store buffers
- // for locations far enough from the header, so that we do not
- // need to walk big objects only to find the single new
- // reference in the last word during scavenge. This doesn't seem
- // to be a problem though currently.
- ASSERT(p != NULL);
- thread_->StoreBufferAddObjectGC(visiting_old_object_);
- }
- }
-
#ifndef PRODUCT
void UpdateLiveOld(intptr_t class_id, intptr_t size) {
// TODO(koda): Support growing the array once mutator runs concurrently.
@@ -404,7 +354,6 @@
PageSpace* page_space_;
MarkerWorkList work_list_;
RawWeakProperty* delayed_weak_properties_;
- RawObject* visiting_old_object_;
SkippedCodeFunctions* skipped_code_functions_;
uintptr_t marked_bytes_;
@@ -459,11 +408,36 @@
void GCMarker::Prologue(Isolate* isolate) {
isolate->PrepareForGC();
- // The store buffers will be rebuilt as part of marking, reset them now.
- isolate->store_buffer()->Reset();
}
-void GCMarker::Epilogue(Isolate* isolate) {}
+void GCMarker::Epilogue(Isolate* isolate) {
+ // Filter collected objects from the remembered set.
+ StoreBuffer* store_buffer = isolate->store_buffer();
+ StoreBufferBlock* reading = store_buffer->Blocks();
+ StoreBufferBlock* writing = store_buffer->PopNonFullBlock();
+ while (reading != NULL) {
+ StoreBufferBlock* next = reading->next();
+ // Generated code appends to store buffers; tell MemorySanitizer.
+ MSAN_UNPOISON(reading, sizeof(*reading));
+ while (!reading->IsEmpty()) {
+ RawObject* raw_object = reading->Pop();
+ ASSERT(!raw_object->IsForwardingCorpse());
+ ASSERT(raw_object->IsRemembered());
+ if (raw_object->IsMarked()) {
+ writing->Push(raw_object);
+ if (writing->IsFull()) {
+ store_buffer->PushBlock(writing, StoreBuffer::kIgnoreThreshold);
+ writing = store_buffer->PopNonFullBlock();
+ }
+ }
+ }
+ reading->Reset();
+ // Return the emptied block for recycling (no need to check threshold).
+ store_buffer->PushBlock(reading, StoreBuffer::kIgnoreThreshold);
+ reading = next;
+ }
+ store_buffer->PushBlock(writing, StoreBuffer::kIgnoreThreshold);
+}
void GCMarker::IterateRoots(Isolate* isolate,
ObjectPointerVisitor* visitor,
