6662086: 6u4+, 7b11+: CMS never clears referents when -XX:+ParallelRefProcEnabled
authorysr
Tue, 06 May 2008 15:37:36 -0700
changeset 390 2e094c1be4af
parent 389 a44227868a4a
child 391 f889070a8684
6662086: 6u4+, 7b11+: CMS never clears referents when -XX:+ParallelRefProcEnabled Summary: Construct the relevant CMSIsAliveClosure used by CMS during parallel reference processing with the correct span. It had incorrectly been constructed with an empty span, a regression introduced in 6417901. Reviewed-by: jcoomes
hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp
hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp	Sun May 04 03:29:31 2008 -0700
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp	Tue May 06 15:37:36 2008 -0700
@@ -329,7 +329,7 @@
 class CMSKeepAliveClosure: public OopClosure {
  private:
   CMSCollector* _collector;
-  MemRegion     _span;
+  const MemRegion _span;
   CMSMarkStack* _mark_stack;
   CMSBitMap*    _bit_map;
  protected:
@@ -340,7 +340,9 @@
     _collector(collector),
     _span(span),
     _bit_map(bit_map),
-    _mark_stack(mark_stack) { }
+    _mark_stack(mark_stack) {
+    assert(!_span.is_empty(), "Empty span could spell trouble");
+  }
   virtual void do_oop(oop* p);
   virtual void do_oop(narrowOop* p);
   inline void do_oop_nv(oop* p)       { CMSKeepAliveClosure::do_oop_work(p); }
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Sun May 04 03:29:31 2008 -0700
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Tue May 06 15:37:36 2008 -0700
@@ -520,7 +520,10 @@
                  -1 /* lock-free */, "No_lock" /* dummy */),
   _modUnionClosure(&_modUnionTable),
   _modUnionClosurePar(&_modUnionTable),
-  _is_alive_closure(&_markBitMap),
+  // Adjust my span to cover old (cms) gen and perm gen
+  _span(cmsGen->reserved()._union(permGen->reserved())),
+  // Construct the is_alive_closure with _span & markBitMap
+  _is_alive_closure(_span, &_markBitMap),
   _restart_addr(NULL),
   _overflow_list(NULL),
   _preserved_oop_stack(NULL),
@@ -572,11 +575,6 @@
   _cmsGen->cmsSpace()->set_collector(this);
   _permGen->cmsSpace()->set_collector(this);
 
-  // Adjust my span to cover old (cms) gen and perm gen
-  _span = _cmsGen->reserved()._union(_permGen->reserved());
-  // Initialize the span of is_alive_closure
-  _is_alive_closure.set_span(_span);
-
   // Allocate MUT and marking bit map
   {
     MutexLockerEx x(_markBitMap.lock(), Mutex::_no_safepoint_check_flag);
@@ -5496,7 +5494,7 @@
   typedef AbstractRefProcTaskExecutor::ProcessTask ProcessTask;
   CMSCollector*          _collector;
   CMSBitMap*             _mark_bit_map;
-  MemRegion              _span;
+  const MemRegion        _span;
   OopTaskQueueSet*       _task_queues;
   ParallelTaskTerminator _term;
   ProcessTask&           _task;
@@ -5513,7 +5511,10 @@
     _collector(collector), _span(span), _mark_bit_map(mark_bit_map),
     _task_queues(task_queues),
     _term(total_workers, task_queues)
-    { }
+    {
+      assert(_collector->_span.equals(_span) && !_span.is_empty(),
+             "Inconsistency in _span");
+    }
 
   OopTaskQueueSet* task_queues() { return _task_queues; }
 
@@ -5530,11 +5531,12 @@
 };
 
 void CMSRefProcTaskProxy::work(int i) {
+  assert(_collector->_span.equals(_span), "Inconsistency in _span");
   CMSParKeepAliveClosure par_keep_alive(_collector, _span,
                                         _mark_bit_map, work_queue(i));
   CMSParDrainMarkingStackClosure par_drain_stack(_collector, _span,
                                                  _mark_bit_map, work_queue(i));
-  CMSIsAliveClosure is_alive_closure(_mark_bit_map);
+  CMSIsAliveClosure is_alive_closure(_span, _mark_bit_map);
   _task.work(i, is_alive_closure, par_keep_alive, par_drain_stack);
   if (_task.marks_oops_alive()) {
     do_work_steal(i, &par_drain_stack, &par_keep_alive,
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp	Sun May 04 03:29:31 2008 -0700
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp	Tue May 06 15:37:36 2008 -0700
@@ -435,23 +435,22 @@
 // if the object is "live" (reachable). Used in weak
 // reference processing.
 class CMSIsAliveClosure: public BoolObjectClosure {
-  MemRegion  _span;
+  const MemRegion  _span;
   const CMSBitMap* _bit_map;
 
   friend class CMSCollector;
- protected:
-  void set_span(MemRegion span) { _span = span; }
  public:
-  CMSIsAliveClosure(CMSBitMap* bit_map):
-    _bit_map(bit_map) { }
-
   CMSIsAliveClosure(MemRegion span,
                     CMSBitMap* bit_map):
     _span(span),
-    _bit_map(bit_map) { }
+    _bit_map(bit_map) {
+    assert(!span.is_empty(), "Empty span could spell trouble");
+  }
+
   void do_object(oop obj) {
     assert(false, "not to be invoked");
   }
+
   bool do_object_b(oop obj);
 };
 
@@ -600,7 +599,7 @@
   // ("Weak") Reference processing support
   ReferenceProcessor*            _ref_processor;
   CMSIsAliveClosure              _is_alive_closure;
-      // keep this textually after _markBitMap; c'tor dependency
+      // keep this textually after _markBitMap and _span; c'tor dependency
 
   ConcurrentMarkSweepThread*     _cmsThread;   // the thread doing the work
   ModUnionClosure    _modUnionClosure;