Address review comments.

Author: peter-hofer

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

@@ -36,6 +36,7 @@
 import com.oracle.svm.core.heap.ReferenceAccess;
 import com.oracle.svm.core.jdk.UninterruptibleUtils;
 import com.oracle.svm.core.option.RuntimeOptionValues;
+import com.oracle.svm.core.thread.VMOperation;
 import com.oracle.svm.core.util.UnsignedUtils;
 import com.oracle.svm.core.util.VMError;
 
@@ -48,18 +49,19 @@ abstract class AbstractCollectionPolicy implements CollectionPolicy {
      * Don't change these values individually without carefully going over their occurrences in
      * HotSpot source code, there are dependencies between them that are not handled in our code.
      */
-    static final int INITIAL_SURVIVOR_RATIO = 8;
-    static final int MIN_SURVIVOR_RATIO = 3;
+    protected static final int INITIAL_SURVIVOR_RATIO = 8;
+    protected static final int MIN_SURVIVOR_RATIO = 3;
+    protected static final int DEFAULT_TIME_WEIGHT = 25; // -XX:AdaptiveTimeWeight
 
     /* Constants to compute defaults for values which can be set through existing options. */
     /** HotSpot: -XX:MaxHeapSize default without ergonomics. */
-    static final UnsignedWord SMALL_HEAP_SIZE = WordFactory.unsigned(96 * 1024 * 1024);
-    static final int NEW_RATIO = 2; // HotSpot: -XX:NewRatio
-    static final int LARGE_MEMORY_MAX_HEAP_PERCENT = 25; // -XX:MaxRAMPercentage
-    static final int SMALL_MEMORY_MAX_HEAP_PERCENT = 50; // -XX:MinRAMPercentage
-    static final double INITIAL_HEAP_MEMORY_PERCENT = 1.5625; // -XX:InitialRAMPercentage
+    protected static final UnsignedWord SMALL_HEAP_SIZE = WordFactory.unsigned(96 * 1024 * 1024);
+    protected static final int NEW_RATIO = 2; // HotSpot: -XX:NewRatio
+    protected static final int LARGE_MEMORY_MAX_HEAP_PERCENT = 25; // -XX:MaxRAMPercentage
+    protected static final int SMALL_MEMORY_MAX_HEAP_PERCENT = 50; // -XX:MinRAMPercentage
+    protected static final double INITIAL_HEAP_MEMORY_PERCENT = 1.5625; // -XX:InitialRAMPercentage
 
-    protected final AdaptiveWeightedAverage avgYoungGenAlignedChunkFraction = new AdaptiveWeightedAverage(AdaptiveCollectionPolicy.ADAPTIVE_TIME_WEIGHT);
+    protected final AdaptiveWeightedAverage avgYoungGenAlignedChunkFraction = new AdaptiveWeightedAverage(DEFAULT_TIME_WEIGHT);
 
     protected UnsignedWord survivorSize;
     protected UnsignedWord edenSize;
@@ -165,7 +167,8 @@ private void updateSizeParametersLocked(SizeParameters params, SizeParameters pr
          */
         survivorSize = UnsignedUtils.min(survivorSize, params.maxSurvivorSize());
         edenSize = UnsignedUtils.min(edenSize, maxEdenSize());
-        oldSize = UnsignedUtils.min(oldSize, sizes.maxOldSize());
+        oldSize = UnsignedUtils.min(oldSize, params.maxOldSize());
+        promoSize = UnsignedUtils.min(promoSize, params.maxOldSize());
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
@@ -187,18 +190,21 @@ public UnsignedWord getMaximumYoungGenerationSize() {
 
     @Override
     public UnsignedWord getCurrentHeapCapacity() {
+        assert VMOperation.isGCInProgress() : "use only during GC";
         guaranteeSizeParametersInitialized();
         return edenSize.add(survivorSize.multiply(2)).add(oldSize);
     }
 
     @Override
     public UnsignedWord getSurvivorSpacesCapacity() {
+        assert VMOperation.isGCInProgress() : "use only during GC";
         guaranteeSizeParametersInitialized();
         return survivorSize;
     }
 
     @Override
     public UnsignedWord getMaximumFreeAlignedChunksSize() {
+        assert VMOperation.isGCInProgress() : "use only during GC";
         guaranteeSizeParametersInitialized();
         /*
          * Keep chunks ready for allocations in eden and for the survivor to-spaces during young
@@ -207,12 +213,13 @@ public UnsignedWord getMaximumFreeAlignedChunksSize() {
          * getCurrentHeapCapacity() to have chunks ready during full GCs as well.
          */
         UnsignedWord total = edenSize.add(survivorSize);
-        float alignedFraction = Math.min(1, Math.max(0, avgYoungGenAlignedChunkFraction.getAverage()));
+        double alignedFraction = Math.min(1, Math.max(0, avgYoungGenAlignedChunkFraction.getAverage()));
         return UnsignedUtils.fromDouble(UnsignedUtils.toDouble(total) * alignedFraction);
     }
 
     @Override
     public int getTenuringAge() {
+        assert VMOperation.isGCInProgress() : "use only during GC";
         return tenuringThreshold;
     }
 

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

@@ -33,10 +33,12 @@
 import com.oracle.svm.core.util.UnsignedUtils;
 
 /**
- * A port of HotSpot's ParallelGC adaptive size policy for throughput and footprint, but without the
- * pause time goals. The relevant methods in this class have been adapted from classes
- * {@code PSAdaptiveSizePolicy} and its base class {@code AdaptiveSizePolicy}. Method and variable
- * names have been kept mostly the same for comparability.
+ * A garbage collection policy that balances throughput and memory footprint.
+ *
+ * Much of this is based on HotSpot's ParallelGC adaptive size policy, but without the pause time
+ * goals. Many methods in this class have been adapted from classes {@code PSAdaptiveSizePolicy} and
+ * its base class {@code AdaptiveSizePolicy}. Method and variable names have been kept mostly the
+ * same for comparability.
  */
 final class AdaptiveCollectionPolicy extends AbstractCollectionPolicy {
 
@@ -47,40 +49,40 @@ final class AdaptiveCollectionPolicy extends AbstractCollectionPolicy {
      * Don't change these values individually without carefully going over their occurrences in
      * HotSpot source code, there are dependencies between them that are not handled in our code.
      */
-    static final int ADAPTIVE_SIZE_POLICY_READY_THRESHOLD = 5;
-    static final int ADAPTIVE_SIZE_DECREMENT_SCALE_FACTOR = 4;
-    static final int ADAPTIVE_SIZE_POLICY_WEIGHT = 10;
-    static final int ADAPTIVE_TIME_WEIGHT = 25;
-    static final boolean USE_ADAPTIVE_SIZE_POLICY_WITH_SYSTEM_GC = false;
-    static final boolean USE_ADAPTIVE_SIZE_DECAY_MAJOR_GC_COST = true;
-    static final double ADAPTIVE_SIZE_MAJOR_GC_DECAY_TIME_SCALE = 10;
-    static final boolean USE_ADAPTIVE_SIZE_POLICY_FOOTPRINT_GOAL = true;
-    static final int THRESHOLD_TOLERANCE = 10;
-    static final int SURVIVOR_PADDING = 3;
-    static final int INITIAL_TENURING_THRESHOLD = 7;
-    static final int PROMOTED_PADDING = 3;
-    static final int TENURED_GENERATION_SIZE_SUPPLEMENT_DECAY = 2;
-    static final int YOUNG_GENERATION_SIZE_SUPPLEMENT_DECAY = 8;
-    static final int PAUSE_PADDING = 1;
+    private static final int ADAPTIVE_TIME_WEIGHT = DEFAULT_TIME_WEIGHT;
+    private static final int ADAPTIVE_SIZE_POLICY_READY_THRESHOLD = 5;
+    private static final int ADAPTIVE_SIZE_DECREMENT_SCALE_FACTOR = 4;
+    private static final int ADAPTIVE_SIZE_POLICY_WEIGHT = 10;
+    private static final boolean USE_ADAPTIVE_SIZE_POLICY_WITH_SYSTEM_GC = false;
+    private static final boolean USE_ADAPTIVE_SIZE_DECAY_MAJOR_GC_COST = true;
+    private static final double ADAPTIVE_SIZE_MAJOR_GC_DECAY_TIME_SCALE = 10;
+    private static final boolean USE_ADAPTIVE_SIZE_POLICY_FOOTPRINT_GOAL = true;
+    private static final int THRESHOLD_TOLERANCE = 10;
+    private static final int SURVIVOR_PADDING = 3;
+    private static final int INITIAL_TENURING_THRESHOLD = 7;
+    private static final int PROMOTED_PADDING = 3;
+    private static final int TENURED_GENERATION_SIZE_SUPPLEMENT_DECAY = 2;
+    private static final int YOUNG_GENERATION_SIZE_SUPPLEMENT_DECAY = 8;
+    private static final int PAUSE_PADDING = 1;
     /**
      * Ratio of mutator wall-clock time to GC wall-clock time. HotSpot's default is 99, i.e.
      * spending 1% of time in GC. We set it to 19, i.e. 5%, to prefer a small footprint.
      */
-    static final int GC_TIME_RATIO = 19;
+    private static final int GC_TIME_RATIO = 19;
     /**
      * Maximum size increment step percentages. We reduce them from HotSpot's default of 20 to avoid
      * growing the heap too eagerly, and to enable {@linkplain #ADAPTIVE_SIZE_USE_COST_ESTIMATORS
      * cost estimators} to resize the heap in smaller steps which might yield improved throughput
      * when larger steps do not.
      */
-    static final int YOUNG_GENERATION_SIZE_INCREMENT = 10;
-    static final int TENURED_GENERATION_SIZE_INCREMENT = 10;
+    private static final int YOUNG_GENERATION_SIZE_INCREMENT = 10;
+    private static final int TENURED_GENERATION_SIZE_INCREMENT = 10;
     /*
      * Supplements to accelerate the expansion of the heap at startup. We do not use them in favor
      * of a small footprint.
      */
-    static final int YOUNG_GENERATION_SIZE_SUPPLEMENT = 0; // HotSpot default: 80
-    static final int TENURED_GENERATION_SIZE_SUPPLEMENT = 0; // HotSpot default: 80
+    private static final int YOUNG_GENERATION_SIZE_SUPPLEMENT = 0; // HotSpot default: 80
+    private static final int TENURED_GENERATION_SIZE_SUPPLEMENT = 0; // HotSpot default: 80
     /**
      * Use least square fitting to estimate if increasing heap sizes will significantly improve
      * throughput. This is intended to limit memory usage once throughput cannot be increased much
@@ -89,18 +91,18 @@ final class AdaptiveCollectionPolicy extends AbstractCollectionPolicy {
      * discounting of old data points, unlike HotSpot's AdaptiveSizeThroughPutPolicy option
      * (disabled by default) which uses linear least-square fitting without discounting.
      */
-    static final boolean ADAPTIVE_SIZE_USE_COST_ESTIMATORS = true;
-    static final int ADAPTIVE_SIZE_POLICY_INITIALIZING_STEPS = ADAPTIVE_SIZE_POLICY_READY_THRESHOLD;
+    private static final boolean ADAPTIVE_SIZE_USE_COST_ESTIMATORS = true;
+    private static final int ADAPTIVE_SIZE_POLICY_INITIALIZING_STEPS = ADAPTIVE_SIZE_POLICY_READY_THRESHOLD;
     /** The minimum increase in throughput in percent for expanding a space by 1% of its size. */
-    static final double ADAPTIVE_SIZE_ESTIMATOR_MIN_SIZE_THROUGHPUT_TRADEOFF = 0.8;
+    private static final double ADAPTIVE_SIZE_ESTIMATOR_MIN_SIZE_THROUGHPUT_TRADEOFF = 0.8;
     /** The effective number of most recent data points used by estimator (exponential decay). */
-    static final int ADAPTIVE_SIZE_COST_ESTIMATORS_HISTORY_LENGTH = ADAPTIVE_TIME_WEIGHT;
+    private static final int ADAPTIVE_SIZE_COST_ESTIMATORS_HISTORY_LENGTH = ADAPTIVE_TIME_WEIGHT;
     /** Threshold for triggering a complete collection after repeated minor collections. */
-    static final int CONSECUTIVE_MINOR_TO_MAJOR_COLLECTION_PAUSE_TIME_RATIO = 2;
+    private static final int CONSECUTIVE_MINOR_TO_MAJOR_COLLECTION_PAUSE_TIME_RATIO = 2;
 
     /* Constants derived from other constants. */
-    static final double THROUGHPUT_GOAL = 1.0 - 1.0 / (1.0 + GC_TIME_RATIO);
-    static final double THRESHOLD_TOLERANCE_PERCENT = 1.0 + THRESHOLD_TOLERANCE / 100.0;
+    private static final double THROUGHPUT_GOAL = 1.0 - 1.0 / (1.0 + GC_TIME_RATIO);
+    private static final double THRESHOLD_TOLERANCE_PERCENT = 1.0 + THRESHOLD_TOLERANCE / 100.0;
 
     private final Timer minorTimer = new Timer("minor/between minor");
     private final AdaptiveWeightedAverage avgMinorGcCost = new AdaptiveWeightedAverage(ADAPTIVE_TIME_WEIGHT);
@@ -381,7 +383,7 @@ public void onCollectionBegin(boolean completeCollection) { // {major,minor}_col
         UnsignedWord youngChunkBytes = GCImpl.getGCImpl().getAccounting().getYoungChunkBytesBefore();
         if (youngChunkBytes.notEqual(0)) {
             UnsignedWord youngAlignedChunkBytes = HeapImpl.getHeapImpl().getYoungGeneration().getAlignedChunkBytes();
-            avgYoungGenAlignedChunkFraction.sample(UnsignedUtils.toFloat(youngAlignedChunkBytes) / UnsignedUtils.toFloat(youngChunkBytes));
+            avgYoungGenAlignedChunkFraction.sample(UnsignedUtils.toDouble(youngAlignedChunkBytes) / UnsignedUtils.toDouble(youngChunkBytes));
         }
 
         timer.reset();
@@ -534,13 +536,13 @@ private static void updateCollectionEndAverages(AdaptiveWeightedAverage costAver
             double cost = 0;
             double mutatorInSeconds = TimeUtils.nanosToSecondsDouble(mutatorNanos);
             double pauseInSeconds = TimeUtils.nanosToSecondsDouble(pauseNanos);
-            pauseAverage.sample((float) pauseInSeconds);
+            pauseAverage.sample(pauseInSeconds);
             if (mutatorInSeconds > 0 && pauseInSeconds > 0) {
                 double intervalInSeconds = mutatorInSeconds + pauseInSeconds;
                 cost = pauseInSeconds / intervalInSeconds;
-                costAverage.sample((float) cost);
+                costAverage.sample(cost);
                 if (intervalSeconds != null) {
-                    intervalSeconds.sample((float) intervalInSeconds);
+                    intervalSeconds.sample(intervalInSeconds);
                 }
             }
             costEstimator.sample(UnsignedUtils.toDouble(sizeBytes), cost);

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

@@ -29,7 +29,7 @@
 import com.oracle.svm.core.util.UnsignedUtils;
 
 /**
- * A weighted average maintains a running, weighted average of some float value.
+ * A weighted average maintains a running, weighted average of some floating-point value.
  *
  * The average is adaptive in that we smooth it for the initial samples; we don't use the weight
  * until we have enough samples for it to be meaningful.
@@ -41,24 +41,24 @@ class AdaptiveWeightedAverage {
 
     private final int weight;
 
-    private float average;
+    private double average;
     private long sampleCount;
     private boolean isOld;
 
     AdaptiveWeightedAverage(int weight) {
-        this(weight, 0.0f);
+        this(weight, 0);
     }
 
-    AdaptiveWeightedAverage(int weight, float avg) {
+    AdaptiveWeightedAverage(int weight, double avg) {
         this.weight = weight;
         this.average = avg;
     }
 
-    public float getAverage() {
+    public double getAverage() {
         return average;
     }
 
-    public void sample(float value) {
+    public void sample(double value) {
         sampleCount++;
         if (!isOld && sampleCount > OLD_THRESHOLD) {
             isOld = true;
@@ -67,10 +67,10 @@ public void sample(float value) {
     }
 
     public final void sample(UnsignedWord value) {
-        sample(UnsignedUtils.toFloat(value));
+        sample(UnsignedUtils.toDouble(value));
     }
 
-    protected float computeAdaptiveAverage(float sample, float avg) {
+    protected double computeAdaptiveAverage(double sample, double avg) {
         /*
          * We smoothen the samples by not using weight directly until we've had enough data to make
          * it meaningful. We'd like the first weight used to be 1, the second to be 1/2, etc until
@@ -84,9 +84,9 @@ protected float computeAdaptiveAverage(float sample, float avg) {
         return expAvg(avg, sample, adaptiveWeight);
     }
 
-    private static float expAvg(float avg, float sample, long adaptiveWeight) {
+    private static double expAvg(double avg, double sample, long adaptiveWeight) {
         assert adaptiveWeight <= 100 : "weight must be a percentage";
-        return (100.0f - adaptiveWeight) * avg / 100.0f + adaptiveWeight * sample / 100.0f;
+        return (100.0 - adaptiveWeight) * avg / 100.0 + adaptiveWeight * sample / 100.0;
     }
 }
 
@@ -100,8 +100,8 @@ class AdaptivePaddedAverage extends AdaptiveWeightedAverage {
     private final int padding;
     private final boolean noZeroDeviations;
 
-    private float paddedAverage;
-    private float deviation;
+    private double paddedAverage;
+    private double deviation;
 
     AdaptivePaddedAverage(int weight, int padding) {
         this(weight, padding, false);
@@ -119,20 +119,20 @@ class AdaptivePaddedAverage extends AdaptiveWeightedAverage {
     }
 
     @Override
-    public void sample(float value) {
+    public void sample(double value) {
         super.sample(value);
-        float average = super.getAverage();
+        double average = super.getAverage();
         if (value != 0 || !noZeroDeviations) {
             deviation = computeAdaptiveAverage(Math.abs(value - average), deviation);
         }
         paddedAverage = average + padding * deviation;
     }
 
-    public float getPaddedAverage() {
+    public double getPaddedAverage() {
         return paddedAverage;
     }
 
-    public float getDeviation() {
+    public double getDeviation() {
         return deviation;
     }
 }

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

@@ -40,10 +40,7 @@
 import com.oracle.svm.core.util.TimeUtils;
 import com.oracle.svm.core.util.VMError;
 
-/**
- * Garbage collection policies. These are referenced by fully-qualified class names and should not
- * be renamed or moved.
- */
+/** Basic/legacy garbage collection policies. */
 final class BasicCollectionPolicies {
     public static class Options {
         @Option(help = "Percentage of total collection time that should be spent on young generation collections.")//

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

@@ -27,7 +27,6 @@
 import org.graalvm.compiler.options.Option;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
-import org.graalvm.nativeimage.hosted.Feature;
 import org.graalvm.word.UnsignedWord;
 
 import com.oracle.svm.core.SubstrateOptions;
@@ -39,12 +38,12 @@
 /** The interface for a garbage collection policy. All sizes are in bytes. */
 public interface CollectionPolicy {
     final class Options {
-        @Option(help = "The garbage collection policy, one of Adaptive, BySpaceAndTime, OnlyCompletely, OnlyIncrementally, NeverCollect, or the fully-qualified name of a custom policy class.")//
-        public static final HostedOptionKey<String> InitialCollectionPolicy = new HostedOptionKey<>(AdaptiveCollectionPolicy.class.getName());
+        @Option(help = "The garbage collection policy, either Adaptive (default) or BySpaceAndTime.")//
+        public static final HostedOptionKey<String> InitialCollectionPolicy = new HostedOptionKey<>("Adaptive");
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    static CollectionPolicy getInitialPolicy(Feature.FeatureAccess access) {
+    static CollectionPolicy getInitialPolicy() {
         if (SubstrateOptions.UseEpsilonGC.getValue()) {
             return new BasicCollectionPolicies.NeverCollect();
         } else if (!SubstrateOptions.useRememberedSet()) {
@@ -69,18 +68,7 @@ static CollectionPolicy getInitialPolicy(Feature.FeatureAccess access) {
             case "NeverCollect":
                 return new BasicCollectionPolicies.NeverCollect();
         }
-        Class<?> policyClass = access.findClassByName(name);
-        if (policyClass == null) {
-            throw UserError.abort("Policy %s does not exist. If it is a custom policy class, it must be a fully qualified class name, which might require quotes or escaping.", name);
-        }
-        if (!CollectionPolicy.class.isAssignableFrom(policyClass)) {
-            throw UserError.abort("Policy %s does not extend %s.", name, CollectionPolicy.class.getTypeName());
-        }
-        try {
-            return (CollectionPolicy) policyClass.getDeclaredConstructor().newInstance();
-        } catch (Exception ex) {
-            throw UserError.abort(ex, "Policy %s cannot be instantiated.", name);
-        }
+        throw UserError.abort("Policy %s does not exist.", name);
     }
 
     String getName();
@@ -119,8 +107,7 @@ static CollectionPolicy getInitialPolicy(Feature.FeatureAccess access) {
 
     /**
      * The current limit for the size of the entire heap, which is less than or equal to
-     * {@link #getMaximumHeapSize}. Outside of the policy, this limit is used only for free space
-     * calculations.
+     * {@link #getMaximumHeapSize}.
      *
      * NOTE: this can currently be exceeded during a collection while copying objects in the old
      * generation.