8201490: Improve concurrent mark keep alive closure performance
Summary: Avoid doing marking work unless absolutely required.
Reviewed-by: sjohanss, kbarrett
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Wed Apr 18 11:36:48 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Wed Apr 18 11:36:48 2018 +0200
@@ -1387,35 +1387,40 @@
virtual void do_oop( oop* p) { do_oop_work(p); }
template <class T> void do_oop_work(T* p) {
- if (!_cm->has_overflown()) {
- _task->deal_with_reference(p);
- _ref_counter--;
-
- if (_ref_counter == 0) {
- // We have dealt with _ref_counter_limit references, pushing them
- // and objects reachable from them on to the local stack (and
- // possibly the global stack). Call G1CMTask::do_marking_step() to
- // process these entries.
- //
- // We call G1CMTask::do_marking_step() in a loop, which we'll exit if
- // there's nothing more to do (i.e. we're done with the entries that
- // were pushed as a result of the G1CMTask::deal_with_reference() calls
- // above) or we overflow.
- //
- // Note: G1CMTask::do_marking_step() can set the G1CMTask::has_aborted()
- // flag while there may still be some work to do. (See the comment at
- // the beginning of G1CMTask::do_marking_step() for those conditions -
- // one of which is reaching the specified time target.) It is only
- // when G1CMTask::do_marking_step() returns without setting the
- // has_aborted() flag that the marking step has completed.
- do {
- double mark_step_duration_ms = G1ConcMarkStepDurationMillis;
- _task->do_marking_step(mark_step_duration_ms,
- false /* do_termination */,
- _is_serial);
- } while (_task->has_aborted() && !_cm->has_overflown());
- _ref_counter = _ref_counter_limit;
- }
+ if (_cm->has_overflown()) {
+ return;
+ }
+ if (!_task->deal_with_reference(p)) {
+ // We did not add anything to the mark bitmap (or mark stack), so there is
+ // no point trying to drain it.
+ return;
+ }
+ _ref_counter--;
+
+ if (_ref_counter == 0) {
+ // We have dealt with _ref_counter_limit references, pushing them
+ // and objects reachable from them on to the local stack (and
+ // possibly the global stack). Call G1CMTask::do_marking_step() to
+ // process these entries.
+ //
+ // We call G1CMTask::do_marking_step() in a loop, which we'll exit if
+ // there's nothing more to do (i.e. we're done with the entries that
+ // were pushed as a result of the G1CMTask::deal_with_reference() calls
+ // above) or we overflow.
+ //
+ // Note: G1CMTask::do_marking_step() can set the G1CMTask::has_aborted()
+ // flag while there may still be some work to do. (See the comment at
+ // the beginning of G1CMTask::do_marking_step() for those conditions -
+ // one of which is reaching the specified time target.) It is only
+ // when G1CMTask::do_marking_step() returns without setting the
+ // has_aborted() flag that the marking step has completed.
+ do {
+ double mark_step_duration_ms = G1ConcMarkStepDurationMillis;
+ _task->do_marking_step(mark_step_duration_ms,
+ false /* do_termination */,
+ _is_serial);
+ } while (_task->has_aborted() && !_cm->has_overflown());
+ _ref_counter = _ref_counter_limit;
}
}
};
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp Wed Apr 18 11:36:48 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp Wed Apr 18 11:36:48 2018 +0200
@@ -107,7 +107,7 @@
// reference processor as the _is_alive_non_header field
class G1CMIsAliveClosure : public BoolObjectClosure {
G1CollectedHeap* _g1h;
- public:
+public:
G1CMIsAliveClosure(G1CollectedHeap* g1h) : _g1h(g1h) { }
bool do_object_b(oop obj);
@@ -778,15 +778,16 @@
void increment_refs_reached() { ++_refs_reached; }
// Grey the object by marking it. If not already marked, push it on
- // the local queue if below the finger.
- // obj is below its region's NTAMS.
- inline void make_reference_grey(oop obj);
+ // the local queue if below the finger. obj is required to be below its region's NTAMS.
+ // Returns whether there has been a mark to the bitmap.
+ inline bool make_reference_grey(oop obj);
// Grey the object (by calling make_grey_reference) if required,
// e.g. obj is below its containing region's NTAMS.
// Precondition: obj is a valid heap object.
+ // Returns true if the reference caused a mark to be set in the next bitmap.
template <class T>
- inline void deal_with_reference(T* p);
+ inline bool deal_with_reference(T* p);
// Scans an object and visits its children.
inline void scan_task_entry(G1TaskQueueEntry task_entry);
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp Wed Apr 18 11:36:48 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp Wed Apr 18 11:36:48 2018 +0200
@@ -193,9 +193,9 @@
task(worker_id)->update_liveness(obj, size);
}
-inline void G1CMTask::make_reference_grey(oop obj) {
+inline bool G1CMTask::make_reference_grey(oop obj) {
if (!_cm->mark_in_next_bitmap(_worker_id, obj)) {
- return;
+ return false;
}
// No OrderAccess:store_load() is needed. It is implicit in the
@@ -233,16 +233,17 @@
push(entry);
}
}
+ return true;
}
template <class T>
-inline void G1CMTask::deal_with_reference(T* p) {
+inline bool G1CMTask::deal_with_reference(T* p) {
increment_refs_reached();
oop const obj = RawAccess<MO_VOLATILE>::oop_load(p);
if (obj == NULL) {
- return;
+ return false;
}
- make_reference_grey(obj);
+ return make_reference_grey(obj);
}
inline void G1ConcurrentMark::mark_in_prev_bitmap(oop p) {