Move allocation stack unmarking outside of pause.
Reduces pause time by moving the allocation stack unmarking outside
of the pause. This is especially helpful for devices which have longer
GC times since these times result in having more things to be
unmarked in the allocation stack.
Bug: 9969166
Change-Id: I570f2213cbdda9d90545b64538e2cbeb0dc32d16
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index bf3e3f0..8a08f08 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -202,16 +202,6 @@
// This second sweep makes sure that we don't have any objects in the live stack which point to
// freed objects. These cause problems since their references may be previously freed objects.
SweepArray(allocation_stack, false);
- } else {
- timings_.NewSplit("UnMarkAllocStack");
- WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
- // The allocation stack contains things allocated since the start of the GC. These may have been
- // marked during this GC meaning they won't be eligible for reclaiming in the next sticky GC.
- // Remove these objects from the mark bitmaps so that they will be eligible for sticky
- // collection.
- heap_->UnMarkAllocStack(GetHeap()->alloc_space_->GetMarkBitmap(),
- GetHeap()->large_object_space_->GetMarkObjects(),
- allocation_stack);
}
return true;
}
@@ -271,6 +261,26 @@
if (!IsConcurrent()) {
ProcessReferences(self);
+ } else {
+ timings_.NewSplit("UnMarkAllocStack");
+ accounting::ObjectStack* allocation_stack = GetHeap()->allocation_stack_.get();
+ WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+ // The allocation stack contains things allocated since the start of the GC. These may have been
+ // marked during this GC meaning they won't be eligible for reclaiming in the next sticky GC.
+ // Remove these objects from the mark bitmaps so that they will be eligible for sticky
+ // collection.
+ // There is a race here which is safely handled. Another thread such as the hprof could
+ // have flushed the alloc stack after we resumed the threads. This is safe however, since
+ // reseting the allocation stack zeros it out with madvise. This means that we will either
+ // read NULLs or attempt to unmark a newly allocated object which will not be marked in the
+ // first place.
+ mirror::Object** end = allocation_stack->End();
+ for (mirror::Object** it = allocation_stack->Begin(); it != end; ++it) {
+ Object* obj = *it;
+ if (obj != NULL) {
+ UnMarkObjectNonNull(obj);
+ }
+ }
}
// Before freeing anything, lets verify the heap.
@@ -333,7 +343,7 @@
}
}
-inline void MarkSweep::MarkObjectNonNullParallel(const Object* obj, bool check_finger) {
+inline void MarkSweep::MarkObjectNonNullParallel(const Object* obj) {
DCHECK(obj != NULL);
if (MarkObjectParallel(obj)) {
while (UNLIKELY(!mark_stack_->AtomicPushBack(const_cast<Object*>(obj)))) {
@@ -343,10 +353,29 @@
}
}
-inline void MarkSweep::MarkObjectNonNull(const Object* obj, bool check_finger) {
+inline void MarkSweep::UnMarkObjectNonNull(const Object* obj) {
+ DCHECK(!IsImmune(obj));
+ // Try to take advantage of locality of references within a space, failing this find the space
+ // the hard way.
+ accounting::SpaceBitmap* object_bitmap = current_mark_bitmap_;
+ if (UNLIKELY(!object_bitmap->HasAddress(obj))) {
+ accounting::SpaceBitmap* new_bitmap = heap_->GetMarkBitmap()->GetContinuousSpaceBitmap(obj);
+ if (LIKELY(new_bitmap != NULL)) {
+ object_bitmap = new_bitmap;
+ } else {
+ MarkLargeObject(obj, false);
+ return;
+ }
+ }
+
+ DCHECK(object_bitmap->HasAddress(obj));
+ object_bitmap->Clear(obj);
+}
+
+inline void MarkSweep::MarkObjectNonNull(const Object* obj) {
DCHECK(obj != NULL);
- if (obj >= immune_begin_ && obj < immune_end_) {
+ if (IsImmune(obj)) {
DCHECK(IsMarked(obj));
return;
}
@@ -359,7 +388,7 @@
if (LIKELY(new_bitmap != NULL)) {
object_bitmap = new_bitmap;
} else {
- MarkLargeObject(obj);
+ MarkLargeObject(obj, true);
return;
}
}
@@ -377,7 +406,7 @@
}
// Rare case, probably not worth inlining since it will increase instruction cache miss rate.
-bool MarkSweep::MarkLargeObject(const Object* obj) {
+bool MarkSweep::MarkLargeObject(const Object* obj, bool set) {
// TODO: support >1 discontinuous space.
space::LargeObjectSpace* large_object_space = GetHeap()->GetLargeObjectsSpace();
accounting::SpaceSetMap* large_objects = large_object_space->GetMarkObjects();
@@ -394,8 +423,11 @@
if (kProfileLargeObjects) {
++large_object_mark_;
}
- large_objects->Set(obj);
- // Don't need to check finger since large objects never have any object references.
+ if (set) {
+ large_objects->Set(obj);
+ } else {
+ large_objects->Clear(obj);
+ }
return true;
}
return false;
@@ -404,7 +436,7 @@
inline bool MarkSweep::MarkObjectParallel(const Object* obj) {
DCHECK(obj != NULL);
- if (obj >= immune_begin_ && obj < immune_end_) {
+ if (IsImmune(obj)) {
DCHECK(IsMarked(obj));
return false;
}
@@ -420,7 +452,7 @@
// TODO: Remove the Thread::Current here?
// TODO: Convert this to some kind of atomic marking?
MutexLock mu(Thread::Current(), large_object_lock_);
- return MarkLargeObject(obj);
+ return MarkLargeObject(obj, true);
}
}
@@ -435,13 +467,13 @@
// need to be added to the mark stack.
void MarkSweep::MarkObject(const Object* obj) {
if (obj != NULL) {
- MarkObjectNonNull(obj, true);
+ MarkObjectNonNull(obj);
}
}
void MarkSweep::MarkRoot(const Object* obj) {
if (obj != NULL) {
- MarkObjectNonNull(obj, false);
+ MarkObjectNonNull(obj);
}
}
@@ -449,21 +481,21 @@
DCHECK(root != NULL);
DCHECK(arg != NULL);
MarkSweep* mark_sweep = reinterpret_cast<MarkSweep*>(arg);
- mark_sweep->MarkObjectNonNullParallel(root, false);
+ mark_sweep->MarkObjectNonNullParallel(root);
}
void MarkSweep::MarkObjectCallback(const Object* root, void* arg) {
DCHECK(root != NULL);
DCHECK(arg != NULL);
MarkSweep* mark_sweep = reinterpret_cast<MarkSweep*>(arg);
- mark_sweep->MarkObjectNonNull(root, false);
+ mark_sweep->MarkObjectNonNull(root);
}
void MarkSweep::ReMarkObjectVisitor(const Object* root, void* arg) {
DCHECK(root != NULL);
DCHECK(arg != NULL);
MarkSweep* mark_sweep = reinterpret_cast<MarkSweep*>(arg);
- mark_sweep->MarkObjectNonNull(root, true);
+ mark_sweep->MarkObjectNonNull(root);
}
void MarkSweep::VerifyRootCallback(const Object* root, void* arg, size_t vreg,
@@ -1319,7 +1351,7 @@
inline bool MarkSweep::IsMarked(const Object* object) const
SHARED_LOCKS_REQUIRED(Locks::heap_bitmap_lock_) {
- if (object >= immune_begin_ && object < immune_end_) {
+ if (IsImmune(object)) {
return true;
}
DCHECK(current_mark_bitmap_ != NULL);
diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h
index 0e96836..e39e2f7 100644
--- a/runtime/gc/collector/mark_sweep.h
+++ b/runtime/gc/collector/mark_sweep.h
@@ -253,13 +253,22 @@
SHARED_LOCKS_REQUIRED(Locks::heap_bitmap_lock_,
Locks::mutator_lock_);
- void MarkObjectNonNull(const mirror::Object* obj, bool check_finger)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
+ void MarkObjectNonNull(const mirror::Object* obj)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
- void MarkObjectNonNullParallel(const mirror::Object* obj, bool check_finger);
+ // Unmarks an object by clearing the bit inside of the corresponding bitmap, or if it is in a
+ // space set, removing the object from the set.
+ void UnMarkObjectNonNull(const mirror::Object* obj)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
- bool MarkLargeObject(const mirror::Object* obj)
+ // Marks an object atomically, safe to use from multiple threads.
+ void MarkObjectNonNullParallel(const mirror::Object* obj);
+
+ // Marks or unmarks a large object based on whether or not set is true. If set is true, then we
+ // mark, otherwise we unmark.
+ bool MarkLargeObject(const mirror::Object* obj, bool set)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
// Returns true if we need to add obj to a mark stack.
@@ -287,6 +296,11 @@
// Expand mark stack to 2x its current size. Thread safe.
void ExpandMarkStack();
+ // Returns true if an object is inside of the immune region (assumed to be marked).
+ bool IsImmune(const mirror::Object* obj) const {
+ return obj >= immune_begin_ && obj < immune_end_;
+ }
+
static void VerifyRootCallback(const mirror::Object* root, void* arg, size_t vreg,
const StackVisitor *visitor);
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 9a9e00c..74f7b1b 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1055,20 +1055,6 @@
}
}
-void Heap::UnMarkAllocStack(accounting::SpaceBitmap* bitmap, accounting::SpaceSetMap* large_objects,
- accounting::ObjectStack* stack) {
- mirror::Object** limit = stack->End();
- for (mirror::Object** it = stack->Begin(); it != limit; ++it) {
- const mirror::Object* obj = *it;
- DCHECK(obj != NULL);
- if (LIKELY(bitmap->HasAddress(obj))) {
- bitmap->Clear(obj);
- } else {
- large_objects->Clear(obj);
- }
- }
-}
-
collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type, GcCause gc_cause,
bool clear_soft_references) {
Thread* self = Thread::Current();
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 54cf287..c29b83e 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -372,11 +372,6 @@
accounting::ObjectStack* stack)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
- // Unmark all the objects in the allocation stack in the specified bitmap.
- void UnMarkAllocStack(accounting::SpaceBitmap* bitmap, accounting::SpaceSetMap* large_objects,
- accounting::ObjectStack* stack)
- EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
-
// Update and mark mod union table based on gc type.
void UpdateAndMarkModUnion(collector::MarkSweep* mark_sweep, base::TimingLogger& timings,
collector::GcType gc_type)
