8201490: Improve concurrent mark keep alive closure performance
authortschatzl
Wed, 18 Apr 2018 11:36:48 +0200
changeset 49809 ef5220d644e3
parent 49808 f1dcdc3cd6b7
child 49810 b5d5e53232ce
8201490: Improve concurrent mark keep alive closure performance Summary: Avoid doing marking work unless absolutely required. Reviewed-by: sjohanss, kbarrett
src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
src/hotspot/share/gc/g1/g1ConcurrentMark.hpp
src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp
--- 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) {