8208611: Refactor SATBMarkQueue filtering to allow GC-specific filters

Add SATBMarkQueueFilter.

Reviewed-by: tschatzl, eosterlund, rkennke

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -52,6 +52,7 @@
 #include "gc/g1/g1RemSet.hpp"
 #include "gc/g1/g1RootClosures.hpp"
 #include "gc/g1/g1RootProcessor.hpp"
+#include "gc/g1/g1SATBMarkQueueFilter.hpp"
 #include "gc/g1/g1StringDedup.hpp"
 #include "gc/g1/g1ThreadLocalData.hpp"
 #include "gc/g1/g1YCTypes.hpp"
@@ -1668,7 +1669,9 @@ jint G1CollectedHeap::initialize() {
   // Perform any initialization actions delegated to the policy.
   g1_policy()->init(this, &_collection_set);
 
-  G1BarrierSet::satb_mark_queue_set().initialize(SATB_Q_CBL_mon,
+  G1SATBMarkQueueFilter* satb_filter = new G1SATBMarkQueueFilter(this);
+  G1BarrierSet::satb_mark_queue_set().initialize(satb_filter,
+                                                 SATB_Q_CBL_mon,
                                                  SATB_Q_FL_lock,
                                                  G1SATBProcessCompletedThreshold,
                                                  Shared_SATB_Q_lock);

src/hotspot/share/gc/g1/g1SATBMarkQueueFilter.cpp

@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "gc/g1/g1CollectedHeap.inline.hpp"
+#include "gc/g1/g1SATBMarkQueueFilter.hpp"
+#include "gc/g1/heapRegion.hpp"
+#include "gc/g1/satbMarkQueue.hpp"
+#include "oops/oop.hpp"
+#include "utilities/debug.hpp"
+#include "utilities/globalDefinitions.hpp"
+
+G1SATBMarkQueueFilter::G1SATBMarkQueueFilter(G1CollectedHeap* g1h) : _g1h(g1h) {}
+
+// Return true if a SATB buffer entry refers to an object that
+// requires marking.
+//
+// The entry must point into the G1 heap.  In particular, it must not
+// be a NULL pointer.  NULL pointers are pre-filtered and never
+// inserted into a SATB buffer.
+//
+// An entry that is below the NTAMS pointer for the containing heap
+// region requires marking. Such an entry must point to a valid object.
+//
+// An entry that is at least the NTAMS pointer for the containing heap
+// region might be any of the following, none of which should be marked.
+//
+// * A reference to an object allocated since marking started.
+//   According to SATB, such objects are implicitly kept live and do
+//   not need to be dealt with via SATB buffer processing.
+//
+// * A reference to a young generation object. Young objects are
+//   handled separately and are not marked by concurrent marking.
+//
+// * A stale reference to a young generation object. If a young
+//   generation object reference is recorded and not filtered out
+//   before being moved by a young collection, the reference becomes
+//   stale.
+//
+// * A stale reference to an eagerly reclaimed humongous object.  If a
+//   humongous object is recorded and then reclaimed, the reference
+//   becomes stale.
+//
+// The stale reference cases are implicitly handled by the NTAMS
+// comparison. Because of the possibility of stale references, buffer
+// processing must be somewhat circumspect and not assume entries
+// in an unfiltered buffer refer to valid objects.
+
+static inline bool requires_marking(const void* entry, G1CollectedHeap* g1h) {
+  // Includes rejection of NULL pointers.
+  assert(g1h->is_in_reserved(entry),
+         "Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry));
+
+  HeapRegion* region = g1h->heap_region_containing(entry);
+  assert(region != NULL, "No region for " PTR_FORMAT, p2i(entry));
+  if (entry >= region->next_top_at_mark_start()) {
+    return false;
+  }
+
+  assert(oopDesc::is_oop(oop(entry), true /* ignore mark word */),
+         "Invalid oop in SATB buffer: " PTR_FORMAT, p2i(entry));
+
+  return true;
+}
+
+static inline bool discard_entry(const void* entry, G1CollectedHeap* g1h) {
+  return !requires_marking(entry, g1h) || g1h->is_marked_next((oop)entry);
+}
+
+// Workaround for not yet having std::bind.
+class G1SATBMarkQueueFilterFn {
+  G1CollectedHeap* _g1h;
+
+public:
+  G1SATBMarkQueueFilterFn(G1CollectedHeap* g1h) : _g1h(g1h) {}
+
+  // Return true if entry should be filtered out (removed), false if
+  // it should be retained.
+  bool operator()(const void* entry) const {
+    return discard_entry(entry, _g1h);
+  }
+};
+
+void G1SATBMarkQueueFilter::filter(SATBMarkQueue* queue) {
+  queue->apply_filter(G1SATBMarkQueueFilterFn(_g1h));
+}

src/hotspot/share/gc/g1/g1SATBMarkQueueFilter.hpp

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_GC_G1_G1SATBMARKQUEUEFILTER_HPP
+#define SHARE_GC_G1_G1SATBMARKQUEUEFILTER_HPP
+
+#include "gc/g1/satbMarkQueue.hpp"
+
+class G1CollectedHeap;
+
+class G1SATBMarkQueueFilter : public SATBMarkQueueFilter {
+  G1CollectedHeap* _g1h;
+
+public:
+  G1SATBMarkQueueFilter(G1CollectedHeap* g1h);
+
+  virtual void filter(SATBMarkQueue* queue);
+};
+
+#endif // SHARE_GC_G1_G1SATBMARKQUEUEFILTER_HPP

src/hotspot/share/gc/g1/satbMarkQueue.cpp

@@ -52,103 +52,9 @@ void SATBMarkQueue::flush() {
   flush_impl();
 }
 
-// Return true if a SATB buffer entry refers to an object that
-// requires marking.
-//
-// The entry must point into the G1 heap.  In particular, it must not
-// be a NULL pointer.  NULL pointers are pre-filtered and never
-// inserted into a SATB buffer.
-//
-// An entry that is below the NTAMS pointer for the containing heap
-// region requires marking. Such an entry must point to a valid object.
-//
-// An entry that is at least the NTAMS pointer for the containing heap
-// region might be any of the following, none of which should be marked.
-//
-// * A reference to an object allocated since marking started.
-//   According to SATB, such objects are implicitly kept live and do
-//   not need to be dealt with via SATB buffer processing.
-//
-// * A reference to a young generation object. Young objects are
-//   handled separately and are not marked by concurrent marking.
-//
-// * A stale reference to a young generation object. If a young
-//   generation object reference is recorded and not filtered out
-//   before being moved by a young collection, the reference becomes
-//   stale.
-//
-// * A stale reference to an eagerly reclaimed humongous object.  If a
-//   humongous object is recorded and then reclaimed, the reference
-//   becomes stale.
-//
-// The stale reference cases are implicitly handled by the NTAMS
-// comparison. Because of the possibility of stale references, buffer
-// processing must be somewhat circumspect and not assume entries
-// in an unfiltered buffer refer to valid objects.
-
-inline bool requires_marking(const void* entry, G1CollectedHeap* heap) {
-  // Includes rejection of NULL pointers.
-  assert(heap->is_in_reserved(entry),
-         "Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry));
-
-  HeapRegion* region = heap->heap_region_containing(entry);
-  assert(region != NULL, "No region for " PTR_FORMAT, p2i(entry));
-  if (entry >= region->next_top_at_mark_start()) {
-    return false;
-  }
-
-  assert(oopDesc::is_oop(oop(entry), true /* ignore mark word */),
-         "Invalid oop in SATB buffer: " PTR_FORMAT, p2i(entry));
-
-  return true;
-}
-
-inline bool retain_entry(const void* entry, G1CollectedHeap* heap) {
-  return requires_marking(entry, heap) && !heap->is_marked_next((oop)entry);
-}
-
-// This method removes entries from a SATB buffer that will not be
-// useful to the concurrent marking threads.  Entries are retained if
-// they require marking and are not already marked. Retained entries
-// are compacted toward the top of the buffer.
-
-void SATBMarkQueue::filter() {
-  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-  void** buf = _buf;
-
-  if (buf == NULL) {
-    // nothing to do
-    return;
-  }
-
-  // Two-fingered compaction toward the end.
-  void** src = &buf[index()];
-  void** dst = &buf[capacity()];
-  assert(src <= dst, "invariant");
-  for ( ; src < dst; ++src) {
-    // Search low to high for an entry to keep.
-    void* entry = *src;
-    if (retain_entry(entry, g1h)) {
-      // Found keeper.  Search high to low for an entry to discard.
-      while (src < --dst) {
-        if (!retain_entry(*dst, g1h)) {
-          *dst = entry;         // Replace discard with keeper.
-          break;
-        }
-      }
-      // If discard search failed (src == dst), the outer loop will also end.
-    }
-  }
-  // dst points to the lowest retained entry, or the end of the buffer
-  // if all the entries were filtered out.
-  set_index(dst - buf);
-}
-
-// This method will first apply the above filtering to the buffer. If
-// post-filtering a large enough chunk of the buffer has been cleared
-// we can re-use the buffer (instead of enqueueing it) and we can just
-// allow the mutator to carry on executing using the same buffer
-// instead of replacing it.
+// This method will first apply filtering to the buffer. If filtering
+// retains a small enough collection in the buffer, we can continue to
+// use the buffer as-is, instead of enqueueing and replacing it.
 
 bool SATBMarkQueue::should_enqueue_buffer() {
   assert(_lock == NULL || _lock->owned_by_self(),
@@ -198,13 +104,17 @@ void SATBMarkQueue::print(const char* name) {
 
 SATBMarkQueueSet::SATBMarkQueueSet() :
   PtrQueueSet(),
-  _shared_satb_queue(this, true /* permanent */) { }
+  _shared_satb_queue(this, true /* permanent */),
+  _filter(NULL)
+{}
 
-void SATBMarkQueueSet::initialize(Monitor* cbl_mon, Mutex* fl_lock,
+void SATBMarkQueueSet::initialize(SATBMarkQueueFilter* filter,
+                                  Monitor* cbl_mon, Mutex* fl_lock,
                                   int process_completed_threshold,
                                   Mutex* lock) {
   PtrQueueSet::initialize(cbl_mon, fl_lock, process_completed_threshold, -1);
   _shared_satb_queue.set_lock(lock);
+  _filter = filter;
 }
 
 void SATBMarkQueueSet::handle_zero_index_for_thread(JavaThread* t) {

src/hotspot/share/gc/g1/satbMarkQueue.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -47,11 +47,15 @@ class SATBMarkQueue: public PtrQueue {
 
 private:
   // Filter out unwanted entries from the buffer.
-  void filter();
+  inline void filter();
 
 public:
   SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent = false);
 
+  // Removes entries from the buffer that are no longer needed.
+  template<typename Filter>
+  inline void apply_filter(Filter filter_out);
+
   // Process queue entries and free resources.
   void flush();
 
@@ -86,8 +90,15 @@ public:
 
 };
 
+class SATBMarkQueueFilter : public CHeapObj<mtGC> {
+public:
+  virtual ~SATBMarkQueueFilter() {}
+  virtual void filter(SATBMarkQueue* queue) = 0;
+};
+
 class SATBMarkQueueSet: public PtrQueueSet {
   SATBMarkQueue _shared_satb_queue;
+  SATBMarkQueueFilter* _filter;
 
 #ifdef ASSERT
   void dump_active_states(bool expected_active);
@@ -97,7 +108,8 @@ class SATBMarkQueueSet: public PtrQueueSet {
 public:
   SATBMarkQueueSet();
 
-  void initialize(Monitor* cbl_mon, Mutex* fl_lock,
+  void initialize(SATBMarkQueueFilter* filter,
+                  Monitor* cbl_mon, Mutex* fl_lock,
                   int process_completed_threshold,
                   Mutex* lock);
 
@@ -109,6 +121,10 @@ public:
   // set itself, has an active value same as expected_active.
   void set_active_all_threads(bool active, bool expected_active);
 
+  void filter(SATBMarkQueue* queue) {
+    _filter->filter(queue);
+  }
+
   // Filter all the currently-active SATB buffers.
   void filter_thread_buffers();
 
@@ -129,4 +145,45 @@ public:
   void abandon_partial_marking();
 };
 
+inline void SATBMarkQueue::filter() {
+  static_cast<SATBMarkQueueSet*>(qset())->filter(this);
+}
+
+// Removes entries from the buffer that are no longer needed, as
+// determined by filter. If e is a void* entry in the buffer,
+// filter_out(e) must be a valid expression whose value is convertible
+// to bool. Entries are removed (filtered out) if the result is true,
+// retained if false.
+template<typename Filter>
+inline void SATBMarkQueue::apply_filter(Filter filter_out) {
+  void** buf = this->_buf;
+
+  if (buf == NULL) {
+    // nothing to do
+    return;
+  }
+
+  // Two-fingered compaction toward the end.
+  void** src = &buf[this->index()];
+  void** dst = &buf[this->capacity()];
+  assert(src <= dst, "invariant");
+  for ( ; src < dst; ++src) {
+    // Search low to high for an entry to keep.
+    void* entry = *src;
+    if (!filter_out(entry)) {
+      // Found keeper.  Search high to low for an entry to discard.
+      while (src < --dst) {
+        if (filter_out(*dst)) {
+          *dst = entry;         // Replace discard with keeper.
+          break;
+        }
+      }
+      // If discard search failed (src == dst), the outer loop will also end.
+    }
+  }
+  // dst points to the lowest retained entry, or the end of the buffer
+  // if all the entries were filtered out.
+  this->set_index(dst - buf);
+}
+
 #endif // SHARE_VM_GC_G1_SATBMARKQUEUE_HPP