changeset 8777:dcf96943d49e

8079082: VerifyNoCSetOopsClosure is derived twice from Closure Summary: Change closure to a function object and update iteration Reviewed-by: mgerdin, ecaspole
author kbarrett
date Mon, 27 Jul 2015 14:05:55 -0400
parents efbd746ff61e
children e8351756255d
files src/share/vm/gc/g1/concurrentMark.cpp src/share/vm/gc/g1/concurrentMark.hpp src/share/vm/gc/g1/concurrentMark.inline.hpp src/share/vm/gc/shared/taskqueue.hpp src/share/vm/gc/shared/taskqueue.inline.hpp
diffstat 5 files changed, 37 insertions(+), 71 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc/g1/concurrentMark.cpp	Mon Jul 27 18:23:35 2015 +0300
+++ b/src/share/vm/gc/g1/concurrentMark.cpp	Mon Jul 27 14:05:55 2015 -0400
@@ -403,14 +403,6 @@
   _saved_index = -1;
 }
 
-void CMMarkStack::oops_do(OopClosure* f) {
-  assert(_saved_index == _index,
-         err_msg("saved index: %d index: %d", _saved_index, _index));
-  for (int i = 0; i < _index; i += 1) {
-    f->do_oop(&_base[i]);
-  }
-}
-
 CMRootRegions::CMRootRegions() :
   _young_list(NULL), _cm(NULL), _scan_in_progress(false),
   _should_abort(false),  _next_survivor(NULL) { }
@@ -2717,53 +2709,26 @@
 }
 
 #ifndef PRODUCT
-enum VerifyNoCSetOopsPhase {
-  VerifyNoCSetOopsStack,
-  VerifyNoCSetOopsQueues
-};
-
-class VerifyNoCSetOopsClosure : public OopClosure, public ObjectClosure  {
+class VerifyNoCSetOops VALUE_OBJ_CLASS_SPEC {
 private:
   G1CollectedHeap* _g1h;
-  VerifyNoCSetOopsPhase _phase;
+  const char* _phase;
   int _info;
 
-  const char* phase_str() {
-    switch (_phase) {
-    case VerifyNoCSetOopsStack:         return "Stack";
-    case VerifyNoCSetOopsQueues:        return "Queue";
-    default:                            ShouldNotReachHere();
-    }
-    return NULL;
-  }
-
-  void do_object_work(oop obj) {
+public:
+  VerifyNoCSetOops(const char* phase, int info = -1) :
+    _g1h(G1CollectedHeap::heap()),
+    _phase(phase),
+    _info(info)
+  { }
+
+  void operator()(oop obj) const {
+    guarantee(obj->is_oop(),
+              err_msg("Non-oop " PTR_FORMAT ", phase: %s, info: %d",
+                      p2i(obj), _phase, _info));
     guarantee(!_g1h->obj_in_cs(obj),
               err_msg("obj: " PTR_FORMAT " in CSet, phase: %s, info: %d",
-                      p2i((void*) obj), phase_str(), _info));
-  }
-
-public:
-  VerifyNoCSetOopsClosure() : _g1h(G1CollectedHeap::heap()) { }
-
-  void set_phase(VerifyNoCSetOopsPhase phase, int info = -1) {
-    _phase = phase;
-    _info = info;
-  }
-
-  virtual void do_oop(oop* p) {
-    oop obj = oopDesc::load_decode_heap_oop(p);
-    do_object_work(obj);
-  }
-
-  virtual void do_oop(narrowOop* p) {
-    // We should not come across narrow oops while scanning marking
-    // stacks
-    ShouldNotReachHere();
-  }
-
-  virtual void do_object(oop obj) {
-    do_object_work(obj);
+                      p2i(obj), _phase, _info));
   }
 };
 
@@ -2773,17 +2738,13 @@
     return;
   }
 
-  VerifyNoCSetOopsClosure cl;
-
   // Verify entries on the global mark stack
-  cl.set_phase(VerifyNoCSetOopsStack);
-  _markStack.oops_do(&cl);
+  _markStack.iterate(VerifyNoCSetOops("Stack"));
 
   // Verify entries on the task queues
-  for (uint i = 0; i < _max_worker_id; i += 1) {
-    cl.set_phase(VerifyNoCSetOopsQueues, i);
+  for (uint i = 0; i < _max_worker_id; ++i) {
     CMTaskQueue* queue = _task_queues->queue(i);
-    queue->oops_do(&cl);
+    queue->iterate(VerifyNoCSetOops("Queue", i));
   }
 
   // Verify the global finger
@@ -2806,7 +2767,7 @@
 
   // Verify the task fingers
   assert(parallel_marking_threads() <= _max_worker_id, "sanity");
-  for (int i = 0; i < (int) parallel_marking_threads(); i += 1) {
+  for (uint i = 0; i < parallel_marking_threads(); ++i) {
     CMTask* task = _tasks[i];
     HeapWord* task_finger = task->finger();
     if (task_finger != NULL && task_finger < _heap_end) {
--- a/src/share/vm/gc/g1/concurrentMark.hpp	Mon Jul 27 18:23:35 2015 +0300
+++ b/src/share/vm/gc/g1/concurrentMark.hpp	Mon Jul 27 14:05:55 2015 -0400
@@ -246,9 +246,10 @@
   // Make sure that we have not added any entries to the stack during GC.
   void note_end_of_gc();
 
-  // iterate over the oops in the mark stack, up to the bound recorded via
-  // the call above.
-  void oops_do(OopClosure* f);
+  // Apply fn to each oop in the mark stack, up to the bound recorded
+  // via one of the above "note" functions.  The mark stack must not
+  // be modified while iterating.
+  template<typename Fn> void iterate(Fn fn);
 };
 
 class ForceOverflowSettings VALUE_OBJ_CLASS_SPEC {
--- a/src/share/vm/gc/g1/concurrentMark.inline.hpp	Mon Jul 27 18:23:35 2015 +0300
+++ b/src/share/vm/gc/g1/concurrentMark.inline.hpp	Mon Jul 27 14:05:55 2015 -0400
@@ -223,6 +223,15 @@
 
 #undef check_mark
 
+template<typename Fn>
+inline void CMMarkStack::iterate(Fn fn) {
+  assert(_saved_index == _index,
+         err_msg("saved index: %d index: %d", _saved_index, _index));
+  for (int i = 0; i < _index; ++i) {
+    fn(_base[i]);
+  }
+}
+
 inline void CMTask::push(oop obj) {
   HeapWord* objAddr = (HeapWord*) obj;
   assert(_g1h->is_in_g1_reserved(objAddr), "invariant");
--- a/src/share/vm/gc/shared/taskqueue.hpp	Mon Jul 27 18:23:35 2015 +0300
+++ b/src/share/vm/gc/shared/taskqueue.hpp	Mon Jul 27 14:05:55 2015 -0400
@@ -295,8 +295,9 @@
   // Delete any resource associated with the queue.
   ~GenericTaskQueue();
 
-  // apply the closure to all elements in the task queue
-  void oops_do(OopClosure* f);
+  // Apply fn to each element in the task queue.  The queue must not
+  // be modified while iterating.
+  template<typename Fn> void iterate(Fn fn);
 
 private:
   // Element array.
--- a/src/share/vm/gc/shared/taskqueue.inline.hpp	Mon Jul 27 18:23:35 2015 +0300
+++ b/src/share/vm/gc/shared/taskqueue.inline.hpp	Mon Jul 27 14:05:55 2015 -0400
@@ -259,20 +259,14 @@
 }
 
 template<class E, MEMFLAGS F, unsigned int N>
-inline void GenericTaskQueue<E, F, N>::oops_do(OopClosure* f) {
-  // tty->print_cr("START OopTaskQueue::oops_do");
+template<class Fn>
+inline void GenericTaskQueue<E, F, N>::iterate(Fn fn) {
   uint iters = size();
   uint index = _bottom;
   for (uint i = 0; i < iters; ++i) {
     index = decrement_index(index);
-    // tty->print_cr("  doing entry %d," INTPTR_T " -> " INTPTR_T,
-    //            index, &_elems[index], _elems[index]);
-    E* t = (E*)&_elems[index];      // cast away volatility
-    oop* p = (oop*)t;
-    assert((*t)->is_oop_or_null(), err_msg("Expected an oop or NULL at " PTR_FORMAT, p2i(*t)));
-    f->do_oop(p);
+    fn(const_cast<E&>(_elems[index])); // cast away volatility
   }
-  // tty->print_cr("END OopTaskQueue::oops_do");
 }