8229420: [Redo] jstat reports incorrect values for OU for CMS GC

Reviewed-by: andrew

8 files changed, 58 insertions, 3 deletions

diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
index e5a3a1106..2a2faa69a 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp
@@ -161,6 +161,8 @@ CompactibleFreeListSpace::CompactibleFreeListSpace(BlockOffsetSharedArray* bs,
     }
     _dictionary->set_par_lock(&_parDictionaryAllocLock);
   }
+
+  _used_stable = 0;
 }
 
 // Like CompactibleSpace forward() but always calls cross_threshold() to
@@ -377,6 +379,14 @@ size_t CompactibleFreeListSpace::used() const {
   return capacity() - free();
 }
 
+size_t CompactibleFreeListSpace::used_stable() const {
+  return _used_stable;
+}
+
+void CompactibleFreeListSpace::recalculate_used_stable() {
+  _used_stable = used();
+}
+
 size_t CompactibleFreeListSpace::free() const {
   // "MT-safe, but not MT-precise"(TM), if you will: i.e.
   // if you do this while the structures are in flux you
@@ -1218,6 +1228,13 @@ HeapWord* CompactibleFreeListSpace::allocate(size_t size) {
     debug_only(fc->mangleAllocated(size));
   }
 
+  // During GC we do not need to recalculate the stable used value for
+  // every allocation in old gen. It is done once at the end of GC instead
+  // for performance reasons.
+  if (!Universe::heap()->is_gc_active()) {
+    recalculate_used_stable();
+  }
+
   return res;
 }
 
diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp
index 7269068f6..121299d4b 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp
@@ -148,6 +148,9 @@ class CompactibleFreeListSpace: public CompactibleSpace {
   // Used to keep track of limit of sweep for the space
   HeapWord* _sweep_limit;
 
+  // Stable value of used().
+  size_t _used_stable;
+
   // Support for compacting cms
   HeapWord* cross_threshold(HeapWord* start, HeapWord* end);
   HeapWord* forward(oop q, size_t size, CompactPoint* cp, HeapWord* compact_top);
@@ -343,6 +346,17 @@ class CompactibleFreeListSpace: public CompactibleSpace {
   // which overestimates the region by returning the entire
   // committed region (this is safe, but inefficient).
 
+  // Returns monotonically increasing stable used space bytes for CMS.
+  // This is required for jstat and other memory monitoring tools
+  // that might otherwise see inconsistent used space values during a garbage
+  // collection, promotion or allocation into compactibleFreeListSpace.
+  // The value returned by this function might be smaller than the
+  // actual value.
+  size_t used_stable() const;
+  // Recalculate and cache the current stable used() value. Only to be called
+  // in places where we can be sure that the result is stable.
+  void recalculate_used_stable();
+
   // Returns a subregion of the space containing all the objects in
   // the space.
   MemRegion used_region() const {
diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
index 1872e72e3..e7228d313 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
@@ -869,6 +869,10 @@ ConcurrentMarkSweepGeneration::unsafe_max_alloc_nogc() const {
   return _cmsSpace->max_alloc_in_words() * HeapWordSize;
 }
 
+size_t ConcurrentMarkSweepGeneration::used_stable() const {
+  return cmsSpace()->used_stable();
+}
+
 size_t ConcurrentMarkSweepGeneration::max_available() const {
   return free() + _virtual_space.uncommitted_size();
 }
@@ -1955,6 +1959,8 @@ void CMSCollector::compute_new_size() {
   FreelistLocker z(this);
   MetaspaceGC::compute_new_size();
   _cmsGen->compute_new_size_free_list();
+  // recalculate CMS used space after CMS collection
+  _cmsGen->cmsSpace()->recalculate_used_stable();
 }
 
 // A work method used by foreground collection to determine
@@ -2768,6 +2774,7 @@ void ConcurrentMarkSweepGeneration::gc_prologue(bool full) {
 
   _capacity_at_prologue = capacity();
   _used_at_prologue = used();
+  _cmsSpace->recalculate_used_stable();
 
   // Delegate to CMScollector which knows how to coordinate between
   // this and any other CMS generations that it is responsible for
@@ -2837,6 +2844,7 @@ void CMSCollector::gc_epilogue(bool full) {
   _eden_chunk_index = 0;
 
   size_t cms_used   = _cmsGen->cmsSpace()->used();
+  _cmsGen->cmsSpace()->recalculate_used_stable();
 
   // update performance counters - this uses a special version of
   // update_counters() that allows the utilization to be passed as a
@@ -3672,6 +3680,7 @@ void CMSCollector::checkpointRootsInitial(bool asynch) {
     _collectorState = Marking;
   }
   SpecializationStats::print();
+  _cmsGen->cmsSpace()->recalculate_used_stable();
 }
 
 void CMSCollector::checkpointRootsInitialWork(bool asynch) {
@@ -5066,10 +5075,12 @@ void CMSCollector::checkpointRootsFinal(bool asynch,
                     Mutex::_no_safepoint_check_flag);
     assert(!init_mark_was_synchronous, "but that's impossible!");
     checkpointRootsFinalWork(asynch, clear_all_soft_refs, false);
+    _cmsGen->cmsSpace()->recalculate_used_stable();
   } else {
     // already have all the locks
     checkpointRootsFinalWork(asynch, clear_all_soft_refs,
                              init_mark_was_synchronous);
+    _cmsGen->cmsSpace()->recalculate_used_stable();
   }
   verify_work_stacks_empty();
   verify_overflow_empty();
@@ -6368,6 +6379,10 @@ void CMSCollector::sweep(bool asynch) {
       // Update heap occupancy information which is used as
       // input to soft ref clearing policy at the next gc.
       Universe::update_heap_info_at_gc();
+
+      // recalculate CMS used space after CMS collection
+      _cmsGen->cmsSpace()->recalculate_used_stable();
+
       _collectorState = Resizing;
     }
   } else {
@@ -6467,6 +6482,7 @@ void ConcurrentMarkSweepGeneration::update_gc_stats(int current_level,
     // Gather statistics on the young generation collection.
     collector()->stats().record_gc0_end(used());
   }
+  _cmsSpace->recalculate_used_stable();
 }
 
 CMSAdaptiveSizePolicy* ConcurrentMarkSweepGeneration::size_policy() {
diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
index a6d06a5dc..a023b9fb9 100644
--- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
@@ -1190,6 +1190,7 @@ class ConcurrentMarkSweepGeneration: public CardGeneration {
   double occupancy() const { return ((double)used())/((double)capacity()); }
   size_t contiguous_available() const;
   size_t unsafe_max_alloc_nogc() const;
+  size_t used_stable() const;
 
   // over-rides
   MemRegion used_region() const;
diff --git a/src/share/vm/gc_implementation/shared/gSpaceCounters.hpp b/src/share/vm/gc_implementation/shared/gSpaceCounters.hpp
index 7a5fdf9ee..b03430e27 100644
--- a/src/share/vm/gc_implementation/shared/gSpaceCounters.hpp
+++ b/src/share/vm/gc_implementation/shared/gSpaceCounters.hpp
@@ -63,7 +63,7 @@ class GSpaceCounters: public CHeapObj<mtGC> {
   }
 
   inline void update_used() {
-    _used->set_value(_gen->used());
+    _used->set_value(_gen->used_stable());
   }
 
   // special version of update_used() to allow the used value to be
@@ -107,7 +107,7 @@ class GenerationUsedHelper : public PerfLongSampleHelper {
     GenerationUsedHelper(Generation* g) : _gen(g) { }
 
     inline jlong take_sample() {
-      return _gen->used();
+      return _gen->used_stable();
     }
 };
 
diff --git a/src/share/vm/memory/generation.cpp b/src/share/vm/memory/generation.cpp
index 3c45a9192..dc4ac0869 100644
--- a/src/share/vm/memory/generation.cpp
+++ b/src/share/vm/memory/generation.cpp
@@ -68,6 +68,12 @@ GenerationSpec* Generation::spec() {
   return gch->_gen_specs[level()];
 }
 
+// This is for CMS. It returns stable monotonic used space size.
+// Remove this when CMS is removed.
+size_t Generation::used_stable() const {
+  return used();
+}
+
 size_t Generation::max_capacity() const {
   return reserved().byte_size();
 }
diff --git a/src/share/vm/memory/generation.hpp b/src/share/vm/memory/generation.hpp
index 63dccb70f..ca3fe949d 100644
--- a/src/share/vm/memory/generation.hpp
+++ b/src/share/vm/memory/generation.hpp
@@ -168,6 +168,7 @@ class Generation: public CHeapObj<mtGC> {
   virtual size_t capacity() const = 0;  // The maximum number of object bytes the
                                         // generation can currently hold.
   virtual size_t used() const = 0;      // The number of used bytes in the gen.
+  virtual size_t used_stable() const;   // The number of used bytes for memory monitoring tools.
   virtual size_t free() const = 0;      // The number of free bytes in the gen.
 
   // Support for java.lang.Runtime.maxMemory(); see CollectedHeap.
diff --git a/src/share/vm/services/memoryPool.hpp b/src/share/vm/services/memoryPool.hpp
index 007366e0b..157b54e73 100644
--- a/src/share/vm/services/memoryPool.hpp
+++ b/src/share/vm/services/memoryPool.hpp
@@ -198,7 +198,7 @@ public:
                                bool support_usage_threshold);
 
   MemoryUsage get_memory_usage();
-  size_t used_in_bytes()            { return _space->used(); }
+  size_t used_in_bytes()            { return _space->used_stable(); }
 };
 #endif // INCLUDE_ALL_GCS