Re-enable concurrent system weak sweeping.
Enabled by disallowing new system weaks during the pause and
re-allowing it after the system weaks have been swept. Reduces
GC pause by ~1ms.
Fixes pause regression caused by fix for
Bug: 10626133
Change-Id: If49d33e7ef19cb728ed3cef5187acfa53b9b05d8
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 167140c..6790144 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -207,10 +207,6 @@
}
ProcessReferences(self);
- {
- ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
- SweepSystemWeaks();
- }
// Only need to do this if we have the card mark verification on, and only during concurrent GC.
if (GetHeap()->verify_missing_card_marks_ || GetHeap()->verify_pre_gc_heap_||
@@ -228,6 +224,12 @@
// Ensure that nobody inserted items in the live stack after we swapped the stacks.
ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
CHECK_GE(live_stack_freeze_size_, GetHeap()->GetLiveStack()->Size());
+
+ // Disallow new system weaks to prevent a race which occurs when someone adds a new system
+ // weak before we sweep them. Since this new system weak may not be marked, the GC may
+ // incorrectly sweep it. This also fixes a race where interning may attempt to return a strong
+ // reference to a string that is about to be swept.
+ Runtime::Current()->DisallowNewSystemWeaks();
return true;
}
@@ -289,14 +291,16 @@
if (!IsConcurrent()) {
ProcessReferences(self);
- {
- ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
- SweepSystemWeaks();
- }
- timings_.StartSplit("PreSweepingGcVerification");
- heap_->PreSweepingGcVerification(this);
- timings_.EndSplit();
- } else {
+ }
+
+ {
+ WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+ SweepSystemWeaks();
+ }
+
+ if (IsConcurrent()) {
+ Runtime::Current()->AllowNewSystemWeaks();
+
base::TimingLogger::ScopedSplit split("UnMarkAllocStack", &timings_);
WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
accounting::ObjectStack* allocation_stack = GetHeap()->allocation_stack_.get();
@@ -1002,13 +1006,7 @@
}
void MarkSweep::SweepJniWeakGlobals(IsMarkedTester is_marked, void* arg) {
- JavaVMExt* vm = Runtime::Current()->GetJavaVM();
- WriterMutexLock mu(Thread::Current(), vm->weak_globals_lock);
- for (const Object** entry : vm->weak_globals) {
- if (!is_marked(*entry, arg)) {
- *entry = kClearedJniWeakGlobal;
- }
- }
+ Runtime::Current()->GetJavaVM()->SweepWeakGlobals(is_marked, arg);
}
struct ArrayMarkedCheck {
@@ -1029,31 +1027,8 @@
return false;
}
-void MarkSweep::SweepSystemWeaksArray(accounting::ObjectStack* allocations) {
- Runtime* runtime = Runtime::Current();
- // The callbacks check
- // !is_marked where is_marked is the callback but we want
- // !IsMarked && IsLive
- // So compute !(!IsMarked && IsLive) which is equal to (IsMarked || !IsLive).
- // Or for swapped (IsLive || !IsMarked).
-
- timings_.StartSplit("SweepSystemWeaksArray");
- ArrayMarkedCheck visitor;
- visitor.live_stack = allocations;
- visitor.mark_sweep = this;
- runtime->GetInternTable()->SweepInternTableWeaks(IsMarkedArrayCallback, &visitor);
- runtime->GetMonitorList()->SweepMonitorList(IsMarkedArrayCallback, &visitor);
- SweepJniWeakGlobals(IsMarkedArrayCallback, &visitor);
- timings_.EndSplit();
-}
-
void MarkSweep::SweepSystemWeaks() {
Runtime* runtime = Runtime::Current();
- // The callbacks check
- // !is_marked where is_marked is the callback but we want
- // !IsMarked && IsLive
- // So compute !(!IsMarked && IsLive) which is equal to (IsMarked || !IsLive).
- // Or for swapped (IsLive || !IsMarked).
timings_.StartSplit("SweepSystemWeaks");
runtime->GetInternTable()->SweepInternTableWeaks(IsMarkedCallback, this);
runtime->GetMonitorList()->SweepMonitorList(IsMarkedCallback, this);
@@ -1087,12 +1062,7 @@
// Verify system weaks, uses a special IsMarked callback which always returns true.
runtime->GetInternTable()->SweepInternTableWeaks(VerifyIsLiveCallback, this);
runtime->GetMonitorList()->SweepMonitorList(VerifyIsLiveCallback, this);
-
- JavaVMExt* vm = runtime->GetJavaVM();
- ReaderMutexLock mu(Thread::Current(), vm->weak_globals_lock);
- for (const Object** entry : vm->weak_globals) {
- VerifyIsLive(*entry);
- }
+ runtime->GetJavaVM()->SweepWeakGlobals(VerifyIsLiveCallback, this);
}
struct SweepCallbackContext {
@@ -1172,7 +1142,6 @@
void MarkSweep::SweepArray(accounting::ObjectStack* allocations, bool swap_bitmaps) {
space::DlMallocSpace* space = heap_->GetAllocSpace();
-
timings_.StartSplit("SweepArray");
// Newly allocated objects MUST be in the alloc space and those are the only objects which we are
// going to free.