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/collector/concurrent_copying-inl.h b/runtime/gc/collector/concurrent_copying-inl.h
index 26f5ad3..64fa434 100644
--- a/runtime/gc/collector/concurrent_copying-inl.h
+++ b/runtime/gc/collector/concurrent_copying-inl.h
@@ -28,6 +28,47 @@
namespace gc {
namespace collector {
+inline mirror::Object* ConcurrentCopying::MarkUnevacFromSpaceRegionOrImmuneSpace(
+ mirror::Object* ref, accounting::ContinuousSpaceBitmap* bitmap) {
+ // For the Baker-style RB, in a rare case, we could incorrectly change the object from white
+ // to gray even though the object has already been marked through. This happens if a mutator
+ // thread gets preempted before the AtomicSetReadBarrierPointer below, GC marks through the
+ // object (changes it from white to gray and back to white), and the thread runs and
+ // incorrectly changes it from white to gray. We need to detect such "false gray" cases and
+ // change the objects back to white at the end of marking.
+ if (kUseBakerReadBarrier) {
+ // Test the bitmap first to reduce the chance of false gray cases.
+ if (bitmap->Test(ref)) {
+ return ref;
+ }
+ }
+ // This may or may not succeed, which is ok because the object may already be gray.
+ bool cas_success = false;
+ if (kUseBakerReadBarrier) {
+ cas_success = ref->AtomicSetReadBarrierPointer(ReadBarrier::WhitePtr(),
+ ReadBarrier::GrayPtr());
+ }
+ if (bitmap->AtomicTestAndSet(ref)) {
+ // Already marked.
+ if (kUseBakerReadBarrier &&
+ cas_success &&
+ // The object could be white here if a thread gets preempted after a success at the
+ // above AtomicSetReadBarrierPointer, GC has marked through it, and the thread runs up
+ // to this point.
+ ref->GetReadBarrierPointer() == ReadBarrier::GrayPtr()) {
+ // Register a "false-gray" object to change it from gray to white at the end of marking.
+ PushOntoFalseGrayStack(ref);
+ }
+ } else {
+ // Newly marked.
+ if (kUseBakerReadBarrier) {
+ DCHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::GrayPtr());
+ }
+ PushOntoMarkStack(ref);
+ }
+ return ref;
+}
+
inline mirror::Object* ConcurrentCopying::Mark(mirror::Object* from_ref) {
if (from_ref == nullptr) {
return nullptr;
@@ -68,21 +109,7 @@
return to_ref;
}
case space::RegionSpace::RegionType::kRegionTypeUnevacFromSpace: {
- // This may or may not succeed, which is ok.
- if (kUseBakerReadBarrier) {
- from_ref->AtomicSetReadBarrierPointer(ReadBarrier::WhitePtr(), ReadBarrier::GrayPtr());
- }
- mirror::Object* to_ref = from_ref;
- if (region_space_bitmap_->AtomicTestAndSet(from_ref)) {
- // Already marked.
- } else {
- // Newly marked.
- if (kUseBakerReadBarrier) {
- DCHECK_EQ(to_ref->GetReadBarrierPointer(), ReadBarrier::GrayPtr());
- }
- PushOntoMarkStack(to_ref);
- }
- return to_ref;
+ return MarkUnevacFromSpaceRegionOrImmuneSpace(from_ref, region_space_bitmap_);
}
case space::RegionSpace::RegionType::kRegionTypeNone:
return MarkNonMoving(from_ref);