Avoid the need for the black color for the baker-style read barrier.
We used to set marked-through non-moving objects to black to
distinguish between an unmarked object and a marked-through
object (both would be white without black). This was to avoid a rare
case where a marked-through (white) object would be incorrectly set to
gray for a second time (and left gray) after it's marked
through (white/unmarked -> gray/marked -> white/marked-through ->
gray/incorrect). If an object is left gray, the invariant would be
broken that all objects are white when GC isn't running. Also, we
needed to have an extra pass over non-moving objects to change them
from black to white after the marking phase.
To avoid the need for the black color, we use a 'false gray' stack to
detect such rare cases and register affected objects on it and change
the objects to white at the end of the marking phase. This saves some
GC time because we can avoid the gray-to-black CAS per non-moving
object as well as the extra pass over non-moving objects.
Ritzperf EAAC (N6):
Avg GC time: 232 -> 183 ms (-21%)
Total GC time: 15.3 -> 14.1 s (-7.7%)
Bug: 12687968
Change-Id: Idb29c3dcb745b094bcf6abc4db646dac9cbd1f71
diff --git a/runtime/gc/reference_queue.cc b/runtime/gc/reference_queue.cc
index 03ab9a1..6088a43 100644
--- a/runtime/gc/reference_queue.cc
+++ b/runtime/gc/reference_queue.cc
@@ -68,31 +68,19 @@
Heap* heap = Runtime::Current()->GetHeap();
if (kUseBakerOrBrooksReadBarrier && heap->CurrentCollectorType() == kCollectorTypeCC &&
heap->ConcurrentCopyingCollector()->IsActive()) {
- // Change the gray ptr we left in ConcurrentCopying::ProcessMarkStackRef() to black or white.
+ // Change the gray ptr we left in ConcurrentCopying::ProcessMarkStackRef() to white.
// We check IsActive() above because we don't want to do this when the zygote compaction
// collector (SemiSpace) is running.
CHECK(ref != nullptr);
collector::ConcurrentCopying* concurrent_copying = heap->ConcurrentCopyingCollector();
- const bool is_moving = concurrent_copying->RegionSpace()->IsInToSpace(ref);
- if (ref->GetReadBarrierPointer() == ReadBarrier::GrayPtr()) {
- if (is_moving) {
- ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::WhitePtr());
- CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr());
- } else {
- ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::BlackPtr());
- CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::BlackPtr());
- }
+ mirror::Object* rb_ptr = ref->GetReadBarrierPointer();
+ if (rb_ptr == ReadBarrier::GrayPtr()) {
+ ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::WhitePtr());
+ CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr());
} else {
- // In ConcurrentCopying::ProcessMarkStackRef() we may leave a black or white Reference in the
- // queue and find it here, which is OK. Check that the color makes sense depending on whether
- // the Reference is moving or not and that the referent has been marked.
- if (is_moving) {
- CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr())
- << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer();
- } else {
- CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::BlackPtr())
- << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer();
- }
+ // In ConcurrentCopying::ProcessMarkStackRef() we may leave a white reference in the queue and
+ // find it here, which is OK.
+ CHECK_EQ(rb_ptr, ReadBarrier::WhitePtr()) << "ref=" << ref << " rb_ptr=" << rb_ptr;
mirror::Object* referent = ref->GetReferent<kWithoutReadBarrier>();
// The referent could be null if it's cleared by a mutator (Reference.clear()).
if (referent != nullptr) {