Always use Set.of for conditions.

Author: vjovanov

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/ClassInitializationInfo.java

@@ -50,35 +50,45 @@
 
 /**
  * Information about the runtime class initialization state of a {@link DynamicHub class}, and
- * {@link #initialize implementation} of class initialization according to the Java VM
- * specification.
- *
+ * {@link #slowPath(ClassInitializationInfo, DynamicHub)} implementation} of class initialization
+ * according to the Java VM specification.
+ * <p>
  * The information is not directly stored in {@link DynamicHub} because 1) the class initialization
  * state is mutable while {@link DynamicHub} must be immutable, and 2) few classes require
  * initialization at runtime so factoring out the information reduces image size.
  */
 @InternalVMMethod
 public final class ClassInitializationInfo {
 
-    /** Singleton for classes that failed to link during image building. */
-    public static ClassInitializationInfo createFailedInfo() {
-        return new ClassInitializationInfo(InitState.InitializationError);
+    private static final ClassInitializationInfo NO_INITIALIZER_NO_TRACKING = new ClassInitializationInfo(InitState.FullyInitialized, false, true, false);
+    private static final ClassInitializationInfo INITIALIZED_NO_TRACKING = new ClassInitializationInfo(InitState.FullyInitialized, true, true, false);
+
+    public static ClassInitializationInfo createFailedInfo(boolean typeReachedTracked) {
+        return new ClassInitializationInfo(InitState.InitializationError, typeReachedTracked);
     }
 
     /**
      * Singleton for classes that are already initialized during image building and do not need
      * class initialization at runtime, and don't have {@code <clinit>} methods.
      */
-    public static ClassInitializationInfo createNoInitializerInfo() {
-        return new ClassInitializationInfo(InitState.FullyInitialized, false, true);
+    public static ClassInitializationInfo createNoInitializerInfo(boolean typeReachedTracked) {
+        if (typeReachedTracked) {
+            return new ClassInitializationInfo(InitState.FullyInitialized, false, true, typeReachedTracked);
+        } else {
+            return NO_INITIALIZER_NO_TRACKING;
+        }
     }
 
     /**
      * For classes that are already initialized during image building and do not need class
      * initialization at runtime, but have {@code <clinit>} methods.
      */
-    public static ClassInitializationInfo createInitializedInfo() {
-        return new ClassInitializationInfo(InitState.FullyInitialized, true, true);
+    public static ClassInitializationInfo createInitializedInfo(boolean typeReachedTracked) {
+        if (typeReachedTracked) {
+            return new ClassInitializationInfo(InitState.FullyInitialized, true, true, typeReachedTracked);
+        } else {
+            return INITIALIZED_NO_TRACKING;
+        }
     }
 
     enum InitState {
@@ -95,6 +105,12 @@ enum InitState {
         InitializationError
     }
 
+    enum ReachedTriState {
+        UNTRACKED,
+        NOT_REACHED,
+        REACHED
+    }
+
     interface ClassInitializerFunctionPointer extends CFunctionPointer {
         @InvokeJavaFunctionPointer
         void invoke();
@@ -106,6 +122,11 @@ interface ClassInitializerFunctionPointer extends CFunctionPointer {
      */
     private final FunctionPointerHolder classInitializer;
 
+    /**
+     * Describes whether this class has been reached at runtime.
+     */
+    private ReachedTriState reached;
+
     /**
      * The current initialization state.
      */
@@ -141,34 +162,37 @@ interface ClassInitializerFunctionPointer extends CFunctionPointer {
     private boolean hasInitializer;
     private boolean buildTimeInitialized;
 
-    private boolean reached;
+    public boolean isReachedOrUntracked() {
+        return reached == ReachedTriState.REACHED || reached == ReachedTriState.UNTRACKED;
+    }
 
     public boolean isReached() {
-        return reached;
+        return reached == ReachedTriState.REACHED;
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    private ClassInitializationInfo(InitState initState, boolean hasInitializer, boolean buildTimeInitialized) {
-        this(initState);
+    private ClassInitializationInfo(InitState initState, boolean hasInitializer, boolean buildTimeInitialized, boolean typeReachedTracked) {
+        this(initState, typeReachedTracked);
         this.hasInitializer = hasInitializer;
         this.buildTimeInitialized = buildTimeInitialized;
-        this.reached = false;
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    private ClassInitializationInfo(InitState initState) {
+    private ClassInitializationInfo(InitState initState, boolean typeReachedTracked) {
         this.classInitializer = null;
         this.initState = initState;
         this.initLock = initState == InitState.FullyInitialized ? null : new ReentrantLock();
         this.hasInitializer = true;
+        this.reached = typeReachedTracked ? ReachedTriState.NOT_REACHED : ReachedTriState.UNTRACKED;
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public ClassInitializationInfo(CFunctionPointer classInitializer) {
+    public ClassInitializationInfo(CFunctionPointer classInitializer, boolean typeReachedTracked) {
         this.classInitializer = classInitializer == null || classInitializer.isNull() ? null : new FunctionPointerHolder(classInitializer);
         this.initState = InitState.Linked;
         this.initLock = new ReentrantLock();
         this.hasInitializer = classInitializer != null;
+        this.reached = typeReachedTracked ? ReachedTriState.NOT_REACHED : ReachedTriState.UNTRACKED;
     }
 
     public boolean hasInitializer() {
@@ -199,18 +223,41 @@ private boolean isReentrantInitialization(IsolateThread thread) {
         return thread.equal(initThread);
     }
 
-    private static void reach(DynamicHub hub) {
+    /**
+     * Marks the hierarchy of <code>hub</code> as reached. The concurrency primitives are not needed
+     * as the external readers are always on the separate thread for which the ordering is not
+     * relevant.
+     * </p>
+     * Note: as an optimization we can stop the traversal when a is already reached: The thread that
+     * wrote the flag has continued up the hierarchy. No need to use memory ordering, in the worst
+     * case the threads will mark parts of the hierarchy twice. This is guaranteed by the fact that
+     * the thread that writes always continues up the hierarchy. If the thread reached a reached
+     * type, that means there is at least one thread that climbed above the current point.
+     */
+    private static void markReached(DynamicHub hub) {
         var current = hub;
         do {
-            current.getClassInitializationInfo().reached = true;
+            if (current.getClassInitializationInfo().reached == ReachedTriState.REACHED) {
+                break;
+            }
+
+            if (current.getClassInitializationInfo().reached != ReachedTriState.UNTRACKED) {
+                current.getClassInitializationInfo().reached = ReachedTriState.REACHED;
+            }
             reachInterfaces(current);
             current = current.getSuperHub();
         } while (current != null);
     }
 
     private static void reachInterfaces(DynamicHub hub) {
         for (DynamicHub superInterface : hub.getInterfaces()) {
-            superInterface.getClassInitializationInfo().reached = true;
+            if (superInterface.getClassInitializationInfo().reached == ReachedTriState.REACHED) {
+                return;
+            }
+
+            if (hub.getClassInitializationInfo().reached != ReachedTriState.UNTRACKED) {
+                superInterface.getClassInitializationInfo().reached = ReachedTriState.REACHED;
+            }
             reachInterfaces(superInterface);
         }
     }
@@ -223,10 +270,15 @@ private static void reachInterfaces(DynamicHub hub) {
      * https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.5
      */
     @SubstrateForeignCallTarget(stubCallingConvention = true)
-    private static void initialize(ClassInitializationInfo info, DynamicHub hub) {
+    private static void slowPath(ClassInitializationInfo info, DynamicHub hub) {
         IsolateThread self = CurrentIsolate.getCurrentThread();
 
-        reach(hub);
+        /*
+         * Types are marked as reached before any initialization is performed. Reason: the results
+         * should be visible in class initializers of the whole hierarchy as they could use
+         * reflection.
+         */
+        markReached(hub);
 
         if (info.isInitialized()) {
             return;

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/EnsureClassInitializedSnippets.java

@@ -54,10 +54,10 @@
 import jdk.graal.compiler.replacements.Snippets;
 
 public final class EnsureClassInitializedSnippets extends SubstrateTemplates implements Snippets {
-    private static final SubstrateForeignCallDescriptor INITIALIZE = SnippetRuntime.findForeignCall(ClassInitializationInfo.class, "initialize", HAS_SIDE_EFFECT, LocationIdentity.any());
+    private static final SubstrateForeignCallDescriptor SLOW_PATH = SnippetRuntime.findForeignCall(ClassInitializationInfo.class, "slowPath", HAS_SIDE_EFFECT, LocationIdentity.any());
 
     public static final SubstrateForeignCallDescriptor[] FOREIGN_CALLS = new SubstrateForeignCallDescriptor[]{
-                    INITIALIZE,
+                    SLOW_PATH,
     };
 
     /**
@@ -73,13 +73,13 @@ private static void ensureClassIsInitializedSnippet(@Snippet.NonNullParameter Dy
          */
         ClassInitializationInfo infoNonNull = (ClassInitializationInfo) PiNode.piCastNonNull(info, SnippetAnchorNode.anchor());
 
-        if (!BranchProbabilityNode.probability(BranchProbabilityNode.EXTREMELY_SLOW_PATH_PROBABILITY, infoNonNull.isInitialized() & infoNonNull.isReached())) {
-            callInitializationRoutine(INITIALIZE, infoNonNull, DynamicHub.toClass(hub));
+        if (!BranchProbabilityNode.probability(BranchProbabilityNode.EXTREMELY_SLOW_PATH_PROBABILITY, infoNonNull.isInitialized() & infoNonNull.isReachedOrUntracked())) {
+            callSlowPath(SLOW_PATH, infoNonNull, DynamicHub.toClass(hub));
         }
     }
 
     @NodeIntrinsic(value = ForeignCallWithExceptionNode.class)
-    private static native void callInitializationRoutine(@ConstantNodeParameter ForeignCallDescriptor descriptor, ClassInitializationInfo info, Class<?> clazz);
+    private static native void callSlowPath(@ConstantNodeParameter ForeignCallDescriptor descriptor, ClassInitializationInfo info, Class<?> clazz);
 
     @SuppressWarnings("unused")
     public static void registerLowerings(OptionValues options, Providers providers, Map<Class<? extends Node>, NodeLoweringProvider<?>> lowerings) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/ConditionalRuntimeValue.java

@@ -25,13 +25,16 @@
 package com.oracle.svm.core.configure;
 
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 
+import com.oracle.svm.core.util.VMError;
+
 public final class ConditionalRuntimeValue<T> {
     /*
      * Intentionally an array to save space in the image heap.
@@ -40,8 +43,16 @@ public final class ConditionalRuntimeValue<T> {
     private boolean satisfied;
     volatile T value;
 
+    @Platforms(Platform.HOSTED_ONLY.class)
     public ConditionalRuntimeValue(Set<Class<?>> conditions, T value) {
-        elements = conditions.toArray(Class[]::new);
+        if (!conditions.isEmpty()) {
+            elements = conditions.toArray(Class[]::new);
+        } else {
+            elements = null;
+            satisfied = true;
+        }
+
+        VMError.guarantee(conditions.stream().noneMatch(c -> c.equals(Object.class)), "java.lang.Object must not be in conditions as it is always true.");
         this.value = value;
     }
 
@@ -51,7 +62,11 @@ public T getValueUnconditionally() {
     }
 
     public Set<Class<?>> getConditions() {
-        return Arrays.stream(elements).collect(Collectors.toSet());
+        if (elements == null) {
+            return new HashSet<>();
+        } else {
+            return Arrays.stream(elements).collect(Collectors.toSet());
+        }
     }
 
     public T getValue(Predicate<Class<?>> conditionSatisfied) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java

@@ -80,39 +80,48 @@ public void registerClass(ConfigurationCondition condition, Class<?> clazz) {
                         (currentValue == NEGATIVE_QUERY && ReflectionUtil.lookupClass(true, name) == null),
                         "Invalid Class.forName value for %s: %s", name, currentValue);
 
-        Class<?> conditionClass = condition.getType();
-        Set<Class<?>> resConditions;
-        if (conditionalRuntimeValue != null) {
-            Set<Class<?>> conditions = conditionalRuntimeValue.getConditions();
-            conditions.add(conditionClass);
-            resConditions = conditions;
-        } else {
-            resConditions = Set.of(conditionClass);
-        }
-
+        var cond = updateConditionalValue(conditionalRuntimeValue, clazz, condition);
         if (currentValue == NEGATIVE_QUERY) {
-            knownClasses.put(name, new ConditionalRuntimeValue<>(resConditions, clazz));
+            knownClasses.put(name, cond);
         } else if (currentValue == null) {
-            knownClasses.put(name, new ConditionalRuntimeValue<>(resConditions, clazz));
+            knownClasses.put(name, cond);
         } else if (currentValue instanceof Class<?>) {
-            knownClasses.put(name, new ConditionalRuntimeValue<>(resConditions, clazz));
+            knownClasses.put(name, cond);
         } else {
-            throw VMError.shouldNotReachHere("Testing");
+            throw VMError.shouldNotReachHere("Other cases must not happen.");
+        }
+    }
+
+    private static ConditionalRuntimeValue<Object> updateConditionalValue(ConditionalRuntimeValue<Object> existingConditionalValue, Object newConditionedValue,
+                    ConfigurationCondition additionalCondition) {
+        Set<Class<?>> resConditions = Set.of();
+        if (!additionalCondition.isAlwaysTrue()) {
+            Class<?> conditionClass = additionalCondition.getType();
+            if (existingConditionalValue != null) {
+                Set<Class<?>> conditions = existingConditionalValue.getConditions();
+                conditions.add(conditionClass);
+                resConditions = Set.of(conditions.toArray(v -> new Class<?>[v]));
+            } else {
+                resConditions = Set.of(conditionClass);
+            }
         }
+        return new ConditionalRuntimeValue<>(resConditions, newConditionedValue);
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public void registerExceptionForClass(ConfigurationCondition cnd, String className, Throwable t) {
-        knownClasses.put(className, new ConditionalRuntimeValue<>(Set.of(cnd.getType()), t));
+    public void registerExceptionForClass(ConfigurationCondition condition, String className, Throwable t) {
+        Set<Class<?>> typeSet = condition.isAlwaysTrue() ? Set.of() : Set.of(condition.getType());
+        knownClasses.put(className, new ConditionalRuntimeValue<>(typeSet, t));
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public void registerNegativeQuery(ConfigurationCondition cnd, String className) {
+    public void registerNegativeQuery(ConfigurationCondition condition, String className) {
         /*
          * If the class is not accessible by the builder class loader, but was already registered
          * through registerClass(Class<?>), we don't overwrite the actual class or exception.
          */
-        knownClasses.putIfAbsent(className, new ConditionalRuntimeValue<>(Set.of(cnd.getType()), NEGATIVE_QUERY));
+        Set<Class<?>> typeSet = condition.isAlwaysTrue() ? Set.of() : Set.of(condition.getType());
+        knownClasses.putIfAbsent(className, new ConditionalRuntimeValue<>(typeSet, NEGATIVE_QUERY));
     }
 
     public Class<?> forNameOrNull(String className, ClassLoader classLoader) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/DynamicHubInitializer.java

@@ -49,6 +49,7 @@
 import com.oracle.svm.hosted.ClassLoaderFeature;
 import com.oracle.svm.hosted.ExceptionSynthesizer;
 import com.oracle.svm.hosted.SVMHost;
+import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
 import com.oracle.svm.hosted.classinitialization.SimulateClassInitializerSupport;
 import com.oracle.svm.util.ReflectionUtil;
 
@@ -191,16 +192,18 @@ private void buildClassInitializationInfo(ImageHeapScanner heapScanner, Analysis
         AnalysisError.guarantee(hub.getClassInitializationInfo() == null, "Class initialization info already computed for %s.", type.toJavaName(true));
         boolean initializedAtBuildTime = SimulateClassInitializerSupport.singleton().trySimulateClassInitializer(bb, type);
         ClassInitializationInfo info;
+        var typeReachedTracked = ClassInitializationSupport.singleton().requiresInitializationNodeForTypeReached(type);
         if (initializedAtBuildTime) {
-            info = type.getClassInitializer() == null ? ClassInitializationInfo.createNoInitializerInfo() : ClassInitializationInfo.createInitializedInfo();
+            info = type.getClassInitializer() == null ? ClassInitializationInfo.createNoInitializerInfo(typeReachedTracked)
+                            : ClassInitializationInfo.createInitializedInfo(typeReachedTracked);
         } else {
-            info = buildRuntimeInitializationInfo(type);
+            info = buildRuntimeInitializationInfo(type, typeReachedTracked);
         }
         hub.setClassInitializationInfo(info);
         heapScanner.rescanField(hub, dynamicHubClassInitializationInfoField);
     }
 
-    private ClassInitializationInfo buildRuntimeInitializationInfo(AnalysisType type) {
+    private ClassInitializationInfo buildRuntimeInitializationInfo(AnalysisType type, boolean typeReachedTracked) {
         assert !type.isInitialized();
         try {
             /*
@@ -213,13 +216,13 @@ private ClassInitializationInfo buildRuntimeInitializationInfo(AnalysisType type
             /* Synthesize a VerifyError to be thrown at run time. */
             AnalysisMethod throwVerifyError = metaAccess.lookupJavaMethod(ExceptionSynthesizer.throwExceptionMethod(VerifyError.class));
             bb.addRootMethod(throwVerifyError, true, "Class initialization error, registered in " + DynamicHubInitializer.class);
-            return new ClassInitializationInfo(new MethodPointer(throwVerifyError));
+            return new ClassInitializationInfo(new MethodPointer(throwVerifyError), typeReachedTracked);
         } catch (Throwable t) {
             /*
              * All other linking errors will be reported as NoClassDefFoundError when initialization
              * is attempted at run time.
              */
-            return ClassInitializationInfo.createFailedInfo();
+            return ClassInitializationInfo.createFailedInfo(typeReachedTracked);
         }
 
         /*
@@ -234,7 +237,7 @@ private ClassInitializationInfo buildRuntimeInitializationInfo(AnalysisType type
             bb.addRootMethod(classInitializer, true, "Class initialization, registered in " + DynamicHubInitializer.class);
             classInitializerFunction = new MethodPointer(classInitializer);
         }
-        return new ClassInitializationInfo(classInitializerFunction);
+        return new ClassInitializationInfo(classInitializerFunction, typeReachedTracked);
     }
 
     class InterfacesEncodingKey {