Change ProcessReferences to not use RecursiveMarkObject.
Calling ProcessMarkStack in RecursiveMarkObject caused a lot of
overhead due to timing logger splits. Changed the logic to be the
same as prior to the reference queue refactoring which involves
calling process mark stack after preserving soft references and
enqueueing finalizer references.
FinalizingGC longest pause is reduced by around 1/2 down to ~300ms.
Benchmark score ~400000 -> ~600000.
Also changed the timing logger splits in the GC to have (Paused) if
the split is a paused part of the GC.
Bug: 12129382
Change-Id: I7476d4f23670b19d70738e2fd48e37ec2f57e9f4
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 006c271..7b9d675 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -179,11 +179,11 @@
TimingLogger::ScopedSplit split("ProcessReferences", &timings_);
WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
GetHeap()->ProcessReferences(timings_, clear_soft_references_, &IsMarkedCallback,
- &RecursiveMarkObjectCallback, this);
+ &MarkObjectCallback, &ProcessMarkStackPausedCallback, this);
}
bool MarkSweep::HandleDirtyObjectsPhase() {
- TimingLogger::ScopedSplit split("HandleDirtyObjectsPhase", &timings_);
+ TimingLogger::ScopedSplit split("(Paused)HandleDirtyObjectsPhase", &timings_);
Thread* self = Thread::Current();
Locks::mutator_lock_->AssertExclusiveHeld(self);
@@ -400,10 +400,9 @@
}
}
-mirror::Object* MarkSweep::RecursiveMarkObjectCallback(mirror::Object* obj, void* arg) {
+mirror::Object* MarkSweep::MarkObjectCallback(mirror::Object* obj, void* arg) {
MarkSweep* mark_sweep = reinterpret_cast<MarkSweep*>(arg);
mark_sweep->MarkObject(obj);
- mark_sweep->ProcessMarkStack(true);
return obj;
}
@@ -546,13 +545,6 @@
reinterpret_cast<MarkSweep*>(arg)->MarkObjectNonNull(*root);
}
-mirror::Object* MarkSweep::MarkObjectCallback(mirror::Object* object, void* arg) {
- DCHECK(object != nullptr);
- DCHECK(arg != nullptr);
- reinterpret_cast<MarkSweep*>(arg)->MarkObjectNonNull(object);
- return object;
-}
-
void MarkSweep::VerifyRootCallback(const Object* root, void* arg, size_t vreg,
const StackVisitor* visitor) {
reinterpret_cast<MarkSweep*>(arg)->VerifyRoot(root, vreg, visitor);
@@ -957,7 +949,7 @@
}
void MarkSweep::ReMarkRoots() {
- timings_.StartSplit("ReMarkRoots");
+ timings_.StartSplit("(Paused)ReMarkRoots");
Runtime::Current()->VisitRoots(MarkRootCallback, this, true, true);
timings_.EndSplit();
}
@@ -1208,6 +1200,11 @@
ScanObjectVisit(obj, visitor);
}
+void MarkSweep::ProcessMarkStackPausedCallback(void* arg) {
+ DCHECK(arg != nullptr);
+ reinterpret_cast<MarkSweep*>(arg)->ProcessMarkStack(true);
+}
+
void MarkSweep::ProcessMarkStackParallel(size_t thread_count) {
Thread* self = Thread::Current();
ThreadPool* thread_pool = GetHeap()->GetThreadPool();
@@ -1231,7 +1228,7 @@
// Scan anything that's on the mark stack.
void MarkSweep::ProcessMarkStack(bool paused) {
- timings_.StartSplit("ProcessMarkStack");
+ timings_.StartSplit(paused ? "(Paused)ProcessMarkStack" : "ProcessMarkStack");
size_t thread_count = GetThreadCount(paused);
if (kParallelProcessMarkStack && thread_count > 1 &&
mark_stack_->Size() >= kMinimumParallelMarkStackSize) {
diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h
index 6a48cf7..963b9ea 100644
--- a/runtime/gc/collector/mark_sweep.h
+++ b/runtime/gc/collector/mark_sweep.h
@@ -176,7 +176,7 @@
SHARED_LOCKS_REQUIRED(Locks::heap_bitmap_lock_,
Locks::mutator_lock_);
- static mirror::Object* RecursiveMarkObjectCallback(mirror::Object* obj, void* arg)
+ static mirror::Object* MarkObjectCallback(mirror::Object* obj, void* arg)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
@@ -185,9 +185,8 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
- static mirror::Object* MarkObjectCallback(mirror::Object* object, void* arg)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
+ static void ProcessMarkStackPausedCallback(void* arg)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
static void MarkRootParallelCallback(mirror::Object** root, void* arg, uint32_t thread_id,
RootType root_type)
diff --git a/runtime/gc/collector/semi_space.cc b/runtime/gc/collector/semi_space.cc
index d64ec61..882867b 100644
--- a/runtime/gc/collector/semi_space.cc
+++ b/runtime/gc/collector/semi_space.cc
@@ -163,7 +163,7 @@
TimingLogger::ScopedSplit split("ProcessReferences", &timings_);
WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
GetHeap()->ProcessReferences(timings_, clear_soft_references_, &MarkedForwardingAddressCallback,
- &RecursiveMarkObjectCallback, this);
+ &MarkObjectCallback, &ProcessMarkStackCallback, this);
}
void SemiSpace::MarkingPhase() {
@@ -310,7 +310,7 @@
}
// Recursively process the mark stack.
- ProcessMarkStack(true);
+ ProcessMarkStack();
}
void SemiSpace::ReclaimPhase() {
@@ -571,13 +571,15 @@
return forward_address;
}
-mirror::Object* SemiSpace::RecursiveMarkObjectCallback(mirror::Object* root, void* arg) {
+void SemiSpace::ProcessMarkStackCallback(void* arg) {
+ DCHECK(arg != nullptr);
+ reinterpret_cast<SemiSpace*>(arg)->ProcessMarkStack();
+}
+
+mirror::Object* SemiSpace::MarkObjectCallback(mirror::Object* root, void* arg) {
DCHECK(root != nullptr);
DCHECK(arg != nullptr);
- SemiSpace* semi_space = reinterpret_cast<SemiSpace*>(arg);
- mirror::Object* ret = semi_space->MarkObject(root);
- semi_space->ProcessMarkStack(true);
- return ret;
+ return reinterpret_cast<SemiSpace*>(arg)->MarkObject(root);
}
void SemiSpace::MarkRootCallback(Object** root, void* arg, uint32_t /*thread_id*/,
@@ -587,12 +589,6 @@
*root = reinterpret_cast<SemiSpace*>(arg)->MarkObject(*root);
}
-Object* SemiSpace::MarkObjectCallback(Object* object, void* arg) {
- DCHECK(object != nullptr);
- DCHECK(arg != nullptr);
- return reinterpret_cast<SemiSpace*>(arg)->MarkObject(object);
-}
-
// Marks all objects in the root set.
void SemiSpace::MarkRoots() {
timings_.StartSplit("MarkRoots");
@@ -680,7 +676,7 @@
}
// Scan anything that's on the mark stack.
-void SemiSpace::ProcessMarkStack(bool paused) {
+void SemiSpace::ProcessMarkStack() {
space::MallocSpace* promo_dest_space = NULL;
accounting::SpaceBitmap* live_bitmap = NULL;
if (generational_ && !whole_heap_collection_) {
@@ -694,7 +690,7 @@
DCHECK(mark_bitmap != nullptr);
DCHECK_EQ(live_bitmap, mark_bitmap);
}
- timings_.StartSplit(paused ? "(paused)ProcessMarkStack" : "ProcessMarkStack");
+ timings_.StartSplit("ProcessMarkStack");
while (!mark_stack_->IsEmpty()) {
Object* obj = mark_stack_->PopBack();
if (generational_ && !whole_heap_collection_ && promo_dest_space->HasAddress(obj)) {
diff --git a/runtime/gc/collector/semi_space.h b/runtime/gc/collector/semi_space.h
index 89fe326..ba97376 100644
--- a/runtime/gc/collector/semi_space.h
+++ b/runtime/gc/collector/semi_space.h
@@ -146,12 +146,12 @@
RootType /*root_type*/)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
- static mirror::Object* MarkObjectCallback(mirror::Object* objecgt, void* arg)
- EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
-
- static mirror::Object* RecursiveMarkObjectCallback(mirror::Object* root, void* arg)
+ static mirror::Object* MarkObjectCallback(mirror::Object* root, void* arg)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
+ static void ProcessMarkStackCallback(void* arg)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::heap_bitmap_lock_);
+
virtual mirror::Object* MarkNonForwardedObject(mirror::Object* obj)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
@@ -174,10 +174,6 @@
// Returns true if we should sweep the space.
virtual bool ShouldSweepSpace(space::ContinuousSpace* space) const;
- // Returns how many threads we should use for the current GC phase based on if we are paused,
- // whether or not we care about pauses.
- size_t GetThreadCount(bool paused) const;
-
// Returns true if an object is inside of the immune region (assumed to be marked).
bool IsImmune(const mirror::Object* obj) const ALWAYS_INLINE {
return obj >= immune_begin_ && obj < immune_end_;
@@ -237,7 +233,7 @@
SHARED_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
// Recursively blackens objects on the mark stack.
- void ProcessMarkStack(bool paused)
+ void ProcessMarkStack()
EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::heap_bitmap_lock_);
void EnqueueFinalizerReferences(mirror::Object** ref)
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 9e5e3ab..8c89cdc 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -610,41 +610,44 @@
struct SoftReferenceArgs {
IsMarkedCallback* is_marked_callback_;
- MarkObjectCallback* recursive_mark_callback_;
+ MarkObjectCallback* mark_callback_;
void* arg_;
};
mirror::Object* Heap::PreserveSoftReferenceCallback(mirror::Object* obj, void* arg) {
SoftReferenceArgs* args = reinterpret_cast<SoftReferenceArgs*>(arg);
// TODO: Not preserve all soft references.
- return args->recursive_mark_callback_(obj, args->arg_);
+ return args->mark_callback_(obj, args->arg_);
}
// Process reference class instances and schedule finalizations.
void Heap::ProcessReferences(TimingLogger& timings, bool clear_soft,
IsMarkedCallback* is_marked_callback,
- MarkObjectCallback* recursive_mark_object_callback, void* arg) {
+ MarkObjectCallback* mark_object_callback,
+ ProcessMarkStackCallback* process_mark_stack_callback, void* arg) {
// Unless we are in the zygote or required to clear soft references with white references,
// preserve some white referents.
if (!clear_soft && !Runtime::Current()->IsZygote()) {
SoftReferenceArgs soft_reference_args;
soft_reference_args.is_marked_callback_ = is_marked_callback;
- soft_reference_args.recursive_mark_callback_ = recursive_mark_object_callback;
+ soft_reference_args.mark_callback_ = mark_object_callback;
soft_reference_args.arg_ = arg;
soft_reference_queue_.PreserveSomeSoftReferences(&PreserveSoftReferenceCallback,
&soft_reference_args);
+ process_mark_stack_callback(arg);
}
- timings.StartSplit("ProcessReferences");
+ timings.StartSplit("(Paused)ProcessReferences");
// Clear all remaining soft and weak references with white referents.
soft_reference_queue_.ClearWhiteReferences(cleared_references_, is_marked_callback, arg);
weak_reference_queue_.ClearWhiteReferences(cleared_references_, is_marked_callback, arg);
timings.EndSplit();
// Preserve all white objects with finalize methods and schedule them for finalization.
- timings.StartSplit("EnqueueFinalizerReferences");
+ timings.StartSplit("(Paused)EnqueueFinalizerReferences");
finalizer_reference_queue_.EnqueueFinalizerReferences(cleared_references_, is_marked_callback,
- recursive_mark_object_callback, arg);
+ mark_object_callback, arg);
+ process_mark_stack_callback(arg);
timings.EndSplit();
- timings.StartSplit("ProcessReferences");
+ timings.StartSplit("(Paused)ProcessReferences");
// Clear all f-reachable soft and weak references with white referents.
soft_reference_queue_.ClearWhiteReferences(cleared_references_, is_marked_callback, arg);
weak_reference_queue_.ClearWhiteReferences(cleared_references_, is_marked_callback, arg);
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 80a5a1a..9b763cb 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -339,7 +339,9 @@
static mirror::Object* PreserveSoftReferenceCallback(mirror::Object* obj, void* arg);
void ProcessReferences(TimingLogger& timings, bool clear_soft,
IsMarkedCallback* is_marked_callback,
- MarkObjectCallback* recursive_mark_object_callback, void* arg)
+ MarkObjectCallback* mark_object_callback,
+ ProcessMarkStackCallback* process_mark_stack_callback,
+ void* arg)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_);
diff --git a/runtime/object_callbacks.h b/runtime/object_callbacks.h
index 6af338b..468ba08 100644
--- a/runtime/object_callbacks.h
+++ b/runtime/object_callbacks.h
@@ -60,6 +60,7 @@
// address the object (if the object didn't move, returns the object input parameter).
typedef mirror::Object* (IsMarkedCallback)(mirror::Object* object, void* arg)
__attribute__((warn_unused_result));
+typedef void (ProcessMarkStackCallback)(void* arg);
} // namespace art