8209345: Merge SATBMarkQueueFilter into SATBMarkQueueSet

Move filter extension protocol to SATBMarkQueueSet.

Reviewed-by: shade, tschatzl, rkennke

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

@@ -52,7 +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/g1SATBMarkQueueSet.hpp"
 #include "gc/g1/g1StringDedup.hpp"
 #include "gc/g1/g1ThreadLocalData.hpp"
 #include "gc/g1/g1YCTypes.hpp"
@@ -1687,8 +1687,7 @@ jint G1CollectedHeap::initialize() {
   // Perform any initialization actions delegated to the policy.
   g1_policy()->init(this, &_collection_set);
 
-  G1SATBMarkQueueFilter* satb_filter = new G1SATBMarkQueueFilter(this);
-  G1BarrierSet::satb_mark_queue_set().initialize(satb_filter,
+  G1BarrierSet::satb_mark_queue_set().initialize(this,
                                                  SATB_Q_CBL_mon,
                                                  SATB_Q_FL_lock,
                                                  G1SATBProcessCompletedThreshold,

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

@@ -1,107 +0,0 @@
-/*
- * 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

@@ -1,41 +0,0 @@
-/*
- * 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/g1SATBMarkQueueSet.cpp

@@ -23,8 +23,24 @@
  */
 
 #include "precompiled.hpp"
+#include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1SATBMarkQueueSet.hpp"
 #include "gc/g1/g1ThreadLocalData.hpp"
+#include "gc/g1/heapRegion.hpp"
+#include "gc/g1/satbMarkQueue.hpp"
+#include "oops/oop.hpp"
+#include "utilities/debug.hpp"
+#include "utilities/globalDefinitions.hpp"
+
+G1SATBMarkQueueSet::G1SATBMarkQueueSet() : _g1h(NULL) {}
+
+void G1SATBMarkQueueSet::initialize(G1CollectedHeap* g1h,
+                                    Monitor* cbl_mon, Mutex* fl_lock,
+                                    int process_completed_threshold,
+                                    Mutex* lock) {
+  SATBMarkQueueSet::initialize(cbl_mon, fl_lock, process_completed_threshold, lock);
+  _g1h = g1h;
+}
 
 void G1SATBMarkQueueSet::handle_zero_index_for_thread(JavaThread* t) {
   G1ThreadLocalData::satb_mark_queue(t).handle_zero_index();
@@ -33,3 +49,77 @@ void G1SATBMarkQueueSet::handle_zero_index_for_thread(JavaThread* t) {
 SATBMarkQueue& G1SATBMarkQueueSet::satb_queue_for_thread(JavaThread* const t) const{
   return G1ThreadLocalData::satb_mark_queue(t);
 }
+
+// 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 G1SATBMarkQueueSet::filter(SATBMarkQueue* queue) {
+  assert(_g1h != NULL, "SATB queue set not initialized");
+  apply_filter(G1SATBMarkQueueFilterFn(_g1h), queue);
+}

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

@@ -27,12 +27,23 @@
 
 #include "gc/g1/satbMarkQueue.hpp"
 
+class G1CollectedHeap;
 class JavaThread;
 
 class G1SATBMarkQueueSet : public SATBMarkQueueSet {
+  G1CollectedHeap* _g1h;
+
 public:
+  G1SATBMarkQueueSet();
+
+  void initialize(G1CollectedHeap* g1h,
+                  Monitor* cbl_mon, Mutex* fl_lock,
+                  int process_completed_threshold,
+                  Mutex* lock);
+
   static void handle_zero_index_for_thread(JavaThread* t);
   virtual SATBMarkQueue& satb_queue_for_thread(JavaThread* const t) const;
+  virtual void filter(SATBMarkQueue* queue);
 };
 
 #endif // SHARE_VM_GC_G1_G1SATBMARKQUEUE_HPP

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

@@ -24,9 +24,9 @@
 
 #include "precompiled.hpp"
 #include "jvm.h"
-#include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/satbMarkQueue.hpp"
 #include "gc/shared/collectedHeap.hpp"
+#include "logging/log.hpp"
 #include "memory/allocation.inline.hpp"
 #include "oops/oop.inline.hpp"
 #include "runtime/mutexLocker.hpp"
@@ -103,17 +103,14 @@ void SATBMarkQueue::print(const char* name) {
 
 SATBMarkQueueSet::SATBMarkQueueSet() :
   PtrQueueSet(),
-  _shared_satb_queue(this, true /* permanent */),
-  _filter(NULL)
+  _shared_satb_queue(this, true /* permanent */)
 {}
 
-void SATBMarkQueueSet::initialize(SATBMarkQueueFilter* filter,
-                                  Monitor* cbl_mon, Mutex* fl_lock,
+void SATBMarkQueueSet::initialize(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;
 }
 
 #ifdef ASSERT

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

@@ -49,13 +49,13 @@ private:
   // Filter out unwanted entries from the buffer.
   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);
 
+public:
+  SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent = false);
+
   // Process queue entries and free resources.
   void flush();
 
@@ -90,29 +90,28 @@ 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);
   void verify_active_states(bool expected_active);
 #endif // ASSERT
 
-public:
+protected:
   SATBMarkQueueSet();
+  ~SATBMarkQueueSet() {}
 
-  void initialize(SATBMarkQueueFilter* filter,
-                  Monitor* cbl_mon, Mutex* fl_lock,
+  template<typename Filter>
+  void apply_filter(Filter filter, SATBMarkQueue* queue) {
+    queue->apply_filter(filter);
+  }
+
+  void initialize(Monitor* cbl_mon, Mutex* fl_lock,
                   int process_completed_threshold,
                   Mutex* lock);
 
+public:
   virtual SATBMarkQueue& satb_queue_for_thread(JavaThread* const t) const = 0;
 
   // Apply "set_active(active)" to all SATB queues in the set. It should be
@@ -121,9 +120,7 @@ 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);
-  }
+  virtual void filter(SATBMarkQueue* queue) = 0;
 
   // Filter all the currently-active SATB buffers.
   void filter_thread_buffers();