[GR-71271] Don't do an incremental GC before a forced full GC with CompactingOldGen.

PullRequest: graal/22581

Author: peter-hofer

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/AdaptiveCollectionPolicy.java

@@ -162,10 +162,13 @@ public String getName() {
     }
 
     @Override
-    public boolean shouldCollectCompletely(boolean followingIncrementalCollection) { // should_attempt_scavenge
+    public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) { // should_attempt_scavenge
         guaranteeSizeParametersInitialized();
 
         boolean collectYoungSeparately = shouldCollectYoungGenSeparately(!SerialGCOptions.useCompactingOldGen());
+        if (forcedCompleteCollection && !collectYoungSeparately) {
+            return true;
+        }
         if (!followingIncrementalCollection && collectYoungSeparately) {
             /*
              * With a copying collector, default to always doing an incremental collection first

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/BasicCollectionPolicies.java

@@ -207,7 +207,7 @@ public void onCollectionEnd(boolean completeCollection, GCCause cause) {
     public static final class OnlyIncrementally extends BasicPolicy {
 
         @Override
-        public boolean shouldCollectCompletely(boolean followingIncrementalCollection) {
+        public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) {
             return false;
         }
 
@@ -220,7 +220,7 @@ public String getName() {
     public static final class OnlyCompletely extends BasicPolicy {
 
         @Override
-        public boolean shouldCollectCompletely(boolean followingIncrementalCollection) {
+        public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) {
             return followingIncrementalCollection || !shouldCollectYoungGenSeparately(false);
         }
 
@@ -238,7 +238,7 @@ public boolean shouldCollectOnAllocation() {
         }
 
         @Override
-        public boolean shouldCollectCompletely(boolean followingIncrementalCollection) {
+        public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) {
             throw VMError.shouldNotReachHere("Collection must not be initiated in the first place");
         }
 
@@ -255,8 +255,12 @@ public String getName() {
     public static final class BySpaceAndTime extends BasicPolicy {
 
         @Override
-        public boolean shouldCollectCompletely(boolean followingIncrementalCollection) {
-            if (!followingIncrementalCollection && shouldCollectYoungGenSeparately(false)) {
+        public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) {
+            boolean collectYoungSeparately = shouldCollectYoungGenSeparately(false);
+            if (forcedCompleteCollection && !collectYoungSeparately) {
+                return true;
+            }
+            if (!followingIncrementalCollection && collectYoungSeparately) {
                 return false;
             }
             return estimateUsedHeapAtNextIncrementalCollection().aboveThan(getMaximumHeapSize()) ||

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/CollectionPolicy.java

@@ -138,8 +138,10 @@ static boolean shouldCollectYoungGenSeparately(boolean defaultValue) {
      * @param followingIncrementalCollection whether an incremental collection has just finished in
      *            the same safepoint. Implementations would typically decide whether to follow up
      *            with a full collection based on whether enough memory was reclaimed.
+     * @param forcedCompleteCollection whether a complete collection will eventually be forced. The
+     *            policy can still return {@code false} to do an incremental collection first.
      */
-    boolean shouldCollectCompletely(boolean followingIncrementalCollection);
+    boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection);
 
     /**
      * The current limit for the size of the entire heap, which is less than or equal to

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/GCImpl.java

@@ -319,7 +319,7 @@ private boolean doCollectImpl(GCCause cause, long initialBeginNanoTime, boolean
 
         ChunkBasedCommittedMemoryProvider.get().beforeGarbageCollection();
 
-        boolean incremental = !forceNoIncremental && !policy.shouldCollectCompletely(false);
+        boolean incremental = !forceNoIncremental && !policy.shouldCollectCompletely(false, forceFullGC);
         boolean outOfMemory = false;
 
         if (incremental) {
@@ -330,7 +330,7 @@ private boolean doCollectImpl(GCCause cause, long initialBeginNanoTime, boolean
                 JfrGCEvents.emitGCPhasePauseEvent(getCollectionEpoch(), "Incremental GC", startTicks);
             }
         }
-        if (!incremental || outOfMemory || forceFullGC || policy.shouldCollectCompletely(incremental)) {
+        if (!incremental || outOfMemory || forceFullGC || policy.shouldCollectCompletely(incremental, forceFullGC)) {
             long beginNanoTime = initialBeginNanoTime;
             if (incremental) {
                 beginNanoTime = System.nanoTime();

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/ProportionateSpacesPolicy.java

@@ -64,10 +64,14 @@ public String getName() {
     }
 
     @Override
-    public boolean shouldCollectCompletely(boolean followingIncrementalCollection) {
+    public boolean shouldCollectCompletely(boolean followingIncrementalCollection, boolean forcedCompleteCollection) {
         guaranteeSizeParametersInitialized();
 
-        if (!followingIncrementalCollection && shouldCollectYoungGenSeparately(false)) {
+        boolean collectYoungSeparately = shouldCollectYoungGenSeparately(false);
+        if (forcedCompleteCollection && !collectYoungSeparately) {
+            return true;
+        }
+        if (!followingIncrementalCollection && collectYoungSeparately) {
             // Note that for non-ParallelGC, HotSpot resets the default of ScavengeBeforeFullGC to
             // false, see GCArguments::initialize.
             return false;