8013184: CMS: Call reset_after_compaction() only if a compaction has been done

Reviewed-by: mgerdin, johnc, tschatzl

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp

@@ -193,7 +193,8 @@ ConcurrentMarkSweepGeneration::ConcurrentMarkSweepGeneration(
      FreeBlockDictionary<FreeChunk>::DictionaryChoice dictionaryChoice) :
   CardGeneration(rs, initial_byte_size, level, ct),
   _dilatation_factor(((double)MinChunkSize)/((double)(CollectedHeap::min_fill_size()))),
-  _debug_collection_type(Concurrent_collection_type)
+  _debug_collection_type(Concurrent_collection_type),
+  _did_compact(false)
 {
   HeapWord* bottom = (HeapWord*) _virtual_space.low();
   HeapWord* end    = (HeapWord*) _virtual_space.high();
@@ -923,8 +924,9 @@ void ConcurrentMarkSweepGeneration::compute_new_size() {
   CardGeneration::compute_new_size();
 
   // Reset again after a possible resizing
-  cmsSpace()->reset_after_compaction();
+  if (did_compact()) {
-
+    cmsSpace()->reset_after_compaction();
+  }
 }
 
 void ConcurrentMarkSweepGeneration::compute_new_size_free_list() {
@@ -1574,6 +1576,8 @@ bool CMSCollector::shouldConcurrentCollect() {
   return false;
 }
 
+void CMSCollector::set_did_compact(bool v) { _cmsGen->set_did_compact(v); }
+
 // Clear _expansion_cause fields of constituent generations
 void CMSCollector::clear_expansion_cause() {
   _cmsGen->clear_expansion_cause();
@@ -1671,7 +1675,6 @@ void CMSCollector::collect(bool   full,
   }
   acquire_control_and_collect(full, clear_all_soft_refs);
   _full_gcs_since_conc_gc++;
-
 }
 
 void CMSCollector::request_full_gc(unsigned int full_gc_count) {
@@ -1853,6 +1856,7 @@ NOT_PRODUCT(
     }
   }
 
+  set_did_compact(should_compact);
   if (should_compact) {
     // If the collection is being acquired from the background
     // collector, there may be references on the discovered
@@ -2714,6 +2718,7 @@ void CMSCollector::gc_epilogue(bool full) {
     Chunk::clean_chunk_pool();
   }
 
+  set_did_compact(false);
   _between_prologue_and_epilogue = false;  // ready for next cycle
 }
 

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp

@@ -604,6 +604,8 @@ class CMSCollector: public CHeapObj<mtGC> {
   ConcurrentMarkSweepPolicy* _collector_policy;
   ConcurrentMarkSweepPolicy* collector_policy() { return _collector_policy; }
 
+  void set_did_compact(bool v);
+
   // XXX Move these to CMSStats ??? FIX ME !!!
   elapsedTimer _inter_sweep_timer;   // time between sweeps
   elapsedTimer _intra_sweep_timer;   // time _in_ sweeps
@@ -1081,6 +1083,10 @@ class ConcurrentMarkSweepGeneration: public CardGeneration {
 
   CollectionTypes _debug_collection_type;
 
+  // True if a compactiing collection was done.
+  bool _did_compact;
+  bool did_compact() { return _did_compact; }
+
   // Fraction of current occupancy at which to start a CMS collection which
   // will collect this generation (at least).
   double _initiating_occupancy;
@@ -1121,6 +1127,8 @@ class ConcurrentMarkSweepGeneration: public CardGeneration {
   // Adaptive size policy
   CMSAdaptiveSizePolicy* size_policy();
 
+  void set_did_compact(bool v) { _did_compact = v; }
+
   bool refs_discovery_is_atomic() const { return false; }
   bool refs_discovery_is_mt()     const {
     // Note: CMS does MT-discovery during the parallel-remark

hotspot/test/gc/concurrentMarkSweep/SystemGCOnForegroundCollector.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2013, 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.
+ */
+
+/**
+ * @test SystemGCOnForegroundCollector
+ * @summary CMS: Call reset_after_compaction() only if a compaction has been done
+ * @bug 8013184
+ * @key gc
+ * @key regression
+ * @library /testlibrary
+ * @run main/othervm SystemGCOnForegroundCollector
+ * @author jon.masamitsu@oracle.com
+ */
+
+import com.oracle.java.testlibrary.*;
+
+public class SystemGCOnForegroundCollector {
+  public static void main(String args[]) throws Exception {
+
+    ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+      "-showversion",
+      "-XX:+UseConcMarkSweepGC",
+      "-XX:MaxTenuringThreshold=1",
+      "-XX:-UseCMSCompactAtFullCollection",
+      ThreePlusMSSystemGC.class.getName()
+      );
+
+    OutputAnalyzer output = new OutputAnalyzer(pb.start());
+
+    output.shouldNotContain("error");
+
+    output.shouldHaveExitValue(0);
+  }
+
+  static class ThreePlusMSSystemGC {
+    public static void main(String [] args) {
+      // From running this test 3 System.gc() were always
+      // enough to see the failure but the cause of the failure
+      // depends on how objects are allocated in the CMS generation
+      // which is non-deterministic.  Use 30 iterations for a more
+      // reliable test.
+      for (int i = 0; i < 30; i++) {
+        System.gc();
+      }
+    }
+  }
+}