[GR-68726] Delay the folding of most @stable fields until the analysis is finished

PullRequest: graal/21863

Author: d-kozak

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/spi/JavaConstantFieldProvider.java

@@ -70,8 +70,11 @@ private static ResolvedJavaField findField(ResolvedJavaType type, String fieldNa
     public <T> T readConstantField(ResolvedJavaField field, ConstantFieldTool<T> tool) {
         if (isStableField(field, tool)) {
             JavaConstant value = tool.readValue();
-            if (value != null && isStableFieldValueConstant(field, value, tool)) {
-                return foldStableArray(value, field, tool);
+            if (value != null) {
+                onStableFieldRead(field, value, tool);
+                if (isStableFieldValueConstant(field, value, tool)) {
+                    return foldStableArray(value, field, tool);
+                }
             }
         }
         if (isFinalField(field, tool)) {
@@ -83,6 +86,17 @@ public <T> T readConstantField(ResolvedJavaField field, ConstantFieldTool<T> too
         return null;
     }
 
+    /**
+     * Hook for subclasses to inspect the {@code value} read from the given {@code field}. The value
+     * can be the default for the given kind (i.e. the field will not actually be folded). The
+     * {@code value} will never be {@code null}, but it may be a {@code JavaConstant} representing
+     * null, which will happen when an object field with the default value is read.
+     */
+    @SuppressWarnings("unused")
+    protected void onStableFieldRead(ResolvedJavaField field, JavaConstant value, ConstantFieldTool<?> tool) {
+
+    }
+
     protected <T> T foldStableArray(JavaConstant value, ResolvedJavaField field, ConstantFieldTool<T> tool) {
         return tool.foldStableArray(value, getArrayDimension(field.getType()), isDefaultStableField(field, tool));
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2025, 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
@@ -101,6 +101,7 @@
 import jdk.graal.compiler.debug.Assertions;
 import jdk.graal.compiler.debug.DebugContext;
 import jdk.graal.compiler.phases.util.Providers;
+import jdk.internal.vm.annotation.Stable;
 import jdk.vm.ci.meta.MetaAccessProvider;
 
 @SuppressWarnings("deprecation")
@@ -563,6 +564,26 @@ public void registerOpaqueMethodReturn(Method method) {
             VMError.guarantee(aMethod.getAllMultiMethods().size() == 1, "Opaque method return called for method with >1 multimethods: %s ", method);
             aMethod.setOpaqueReturn();
         }
+
+        /**
+         * Calling this method allows a given {@link Stable} {@code field} to be folded before
+         * analysis, which may improve analysis precision and allow more classes to be initialized
+         * via simulation. Users of this method are <b>required to somehow initialize the field
+         * before analysis</b>, ideally next to the call to {@code allowStableFieldFolding} or at
+         * least write a comment there to explain how the {@code field} is initialized. Trying to
+         * fold such a {@link Stable} field which still has a default value by the time the analysis
+         * is started results in a <b>build failure</b>, because if the initialization occurs later,
+         * the folding might result in a non-deterministic behavior, as the field will get folded
+         * before/during analysis only in some builds when the initialization happened fast enough,
+         * resulting in unstable number of reachable methods and unstable decisions of the
+         * simulation of class initializers.
+         * 
+         * @see SVMHost#allowStableFieldFoldingBeforeAnalysis
+         */
+        public void allowStableFieldFoldingBeforeAnalysis(Field field) {
+            VMError.guarantee(field.isAnnotationPresent(Stable.class), "This method should only be called for @Stable fields: %s", field);
+            getHostVM().allowStableFieldFoldingBeforeAnalysis(getMetaAccess().lookupJavaField(field));
+        }
     }
 
     public static class DuringAnalysisAccessImpl extends BeforeAnalysisAccessImpl implements Feature.DuringAnalysisAccess {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

@@ -814,6 +814,7 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
                 BeforeAnalysisAccessImpl config = new BeforeAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
                 ServiceCatalogSupport.singleton().enableServiceCatalogMapTransformer(config);
                 featureHandler.forEachFeature(feature -> feature.beforeAnalysis(config));
+                bb.getHostVM().checkWellKnownStableFieldsBeforeAnalysis(bb);
                 ServiceCatalogSupport.singleton().seal();
                 bb.getHostVM().getClassInitializationSupport().sealConfiguration();
                 if (ImageLayerBuildingSupport.buildingImageLayer()) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java

@@ -167,6 +167,7 @@
 import jdk.internal.reflect.Reflection;
 import jdk.internal.vm.annotation.DontInline;
 import jdk.internal.vm.annotation.ForceInline;
+import jdk.internal.vm.annotation.Stable;
 import jdk.vm.ci.meta.DeoptimizationReason;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.MetaAccessProvider;
@@ -206,6 +207,15 @@ public enum UsageKind {
     private final ConcurrentMap<AnalysisMethod, Boolean> analysisTrivialMethods = new ConcurrentHashMap<>();
 
     private final Set<AnalysisField> finalFieldsInitializedOutsideOfConstructor = ConcurrentHashMap.newKeySet();
+    /**
+     * We do not allow the folding of arbitrary {@link Stable} fields if the analysis has not
+     * finished yet as it leads to non-deterministic results, because the initialization code often
+     * runs in parallel with the analysis, creating race conditions. However, we need to allow the
+     * folding of {@link Stable} fields in some cases to improve analysis precision. If we know that
+     * such fields are initialized before parsing any method that reads their values, the analysis
+     * results should still be deterministic.
+     */
+    private final Set<AnalysisField> stableFieldsToFoldBeforeAnalysis = ConcurrentHashMap.newKeySet();
     private final MultiMethodAnalysisPolicy multiMethodAnalysisPolicy;
     private final SVMParsingSupport parsingSupport;
     private final InlineBeforeAnalysisPolicy inlineBeforeAnalysisPolicy;
@@ -1298,11 +1308,65 @@ public void recordFieldStore(ResolvedJavaField field, ResolvedJavaMethod method)
         }
     }
 
+    /**
+     * Check the set of well-known {@link Stable} fields and if any of them is already initialized
+     * (has a non-default value), allow its constant folding. This method should be called <b>before
+     * analysis</b> but after all {@link Feature#beforeAnalysis} callbacks finished to give features
+     * a chance to initialize the {@link Stable} fields.
+     * 
+     * @see #stableFieldsToFoldBeforeAnalysis
+     * 
+     * @implNote The "set" is currently only a single field {@code Unsafe.memoryAccessWarned}, but
+     *           we may extend that in the future.
+     */
+    public void checkWellKnownStableFieldsBeforeAnalysis(BigBang bb) {
+        assert !BuildPhaseProvider.isAnalysisStarted() : "This method should be called before the analysis is started.";
+        /*
+         * Folding of Unsafe.memoryAccessWarned is important to remove the code that prints the
+         * warning when unsafe is used for the first time (see Unsafe.beforeMemoryAccess), which
+         * would otherwise be marked as reachable. Unsafe.memoryAccessWarned is not initialized
+         * early enough in every build, but if it is already initialized by this point, it is
+         * beneficial to fold it.
+         */
+        var field = ReflectionUtil.lookupField(loader.findClassOrFail("sun.misc.Unsafe"), "memoryAccessWarned");
+        try {
+            var memoryAccessWarned = (boolean) field.get(null);
+            if (memoryAccessWarned) {
+                allowStableFieldFoldingBeforeAnalysis(bb.getMetaAccess().lookupJavaField(field));
+            }
+        } catch (IllegalAccessException e) {
+            throw VMError.shouldNotReachHere(e);
+        }
+    }
+
+    /**
+     * @see #stableFieldsToFoldBeforeAnalysis
+     */
     public boolean allowConstantFolding(ResolvedJavaField field) {
         AnalysisField aField = field instanceof HostedField ? ((HostedField) field).getWrapped() : (AnalysisField) field;
+        if (!BuildPhaseProvider.isAnalysisFinished() && !aField.isFinal() && aField.isAnnotationPresent(Stable.class)) {
+            return stableFieldsToFoldBeforeAnalysis.contains(aField);
+        }
         return !finalFieldsInitializedOutsideOfConstructor.contains(aField);
     }
 
+    /**
+     * Allows the given {@link Stable} field to be folded before analysis. Use with caution and only
+     * when you know that the given field is initialized before any method that reads its value is
+     * parsed, because trying to fold such a {@link Stable} field which still has a default value by
+     * the time the analysis is started results in a <b>build failure</b>, because if the
+     * initialization occurs later, the folding might result in <i>non-deterministic behavior</i>,
+     * as the field will get folded before/during analysis only in some builds when the
+     * initialization happened fast enough, resulting in unstable number of reachable methods and
+     * unstable decisions of the simulation of class initializers.
+     *
+     * @see #stableFieldsToFoldBeforeAnalysis
+     * @see SimulateClassInitializerSupport
+     */
+    public void allowStableFieldFoldingBeforeAnalysis(AnalysisField field) {
+        stableFieldsToFoldBeforeAnalysis.add(field);
+    }
+
     @Override
     public Object parseGraph(BigBang bb, DebugContext debug, AnalysisMethod method) {
         if (parsingSupport != null) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ameta/AnalysisConstantFieldProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2025, 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
@@ -24,14 +24,24 @@
  */
 package com.oracle.svm.hosted.ameta;
 
+import java.lang.invoke.VarHandle;
+
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 
+import com.oracle.graal.pointsto.heap.ImageHeapConstant;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.svm.core.BuildPhaseProvider;
+import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.SVMHost;
+import com.oracle.svm.hosted.classinitialization.SimulateClassInitializerSupport;
+import com.oracle.svm.hosted.jdk.JDKInitializationFeature;
+import com.oracle.svm.hosted.jdk.VarHandleFeature;
 import com.oracle.svm.hosted.meta.SharedConstantFieldProvider;
 
+import jdk.graal.compiler.virtual.phases.ea.PartialEscapePhase;
+import jdk.internal.vm.annotation.Stable;
+import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaField;
 
@@ -56,6 +66,48 @@ public <T> T readConstantField(ResolvedJavaField f, ConstantFieldTool<T> analysi
         return foldedValue;
     }
 
+    /**
+     * Before analysis, we fold only explicitly registered {@link Stable} fields (see
+     * {@link SVMHost#allowStableFieldFoldingBeforeAnalysis}) that should always be initialized by
+     * then (contain a non-default value).
+     * <p>
+     * There are two edge cases:
+     * <p>
+     * 1) {@code Module#enableNativeAccess}: We optimistically try to fold this field even though we
+     * cannot fold it for every Module, because it significantly reduces the size of smaller images
+     * like helloworld, as discussed in {@link JDKInitializationFeature#beforeAnalysis}. Folding
+     * this particular field only on some accesses does not result in non-determinism, because in
+     * those cases where it is set at build time, it happens before analysis.
+     * <p>
+     * 2) We allow folding default values from virtualized objects. If an object is virtualized by
+     * {@link PartialEscapePhase}, its fields cannot be reassigned, so it is safe to fold its
+     * fields. This sometimes happens with {@link VarHandle}s. Fields whose
+     * {@link ImageHeapConstant#isBackedByHostedObject} returns {@code false}, are the result of
+     * class initializer simulation.
+     *
+     * @see SVMHost#allowConstantFolding(ResolvedJavaField)
+     * @see JDKInitializationFeature#beforeAnalysis
+     * @see PartialEscapePhase
+     * @see VarHandleFeature
+     * @see SimulateClassInitializerSupport
+     */
+    @Override
+    protected void onStableFieldRead(ResolvedJavaField field, JavaConstant value, ConstantFieldTool<?> tool) {
+        if (value.isDefaultForKind() && !BuildPhaseProvider.isAnalysisFinished() && !field.isFinal() && field.isAnnotationPresent(Stable.class)) {
+            if (field.getName().equals("enableNativeAccess") && field.getDeclaringClass().getName().equals("Ljava/lang/Module;")) {
+                /* Edge case 1) */
+                return;
+            }
+            if (!field.isStatic() && tool.getReceiver() instanceof ImageHeapConstant heapConstant && !heapConstant.isBackedByHostedObject()) {
+                /* Edge case 2) */
+                return;
+            }
+            throw VMError.shouldNotReachHere("Attempting to fold an uninitialized @Stable field before analysis: '%s'. " +
+                            "This suggests that the code that is supposed to initialize this field was not executed yet. " +
+                            "Please ensure that the initialization code for the field runs before any method that accesses it is parsed.", field.format("%H.%n"));
+        }
+    }
+
     @Override
     protected AnalysisField asAnalysisField(ResolvedJavaField field) {
         return (AnalysisField) field;

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java

@@ -25,6 +25,7 @@
 package com.oracle.svm.hosted.jdk;
 
 import java.lang.reflect.Field;
+import java.security.CodeSource;
 
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
@@ -35,6 +36,8 @@
 import com.oracle.svm.core.ParsingReason;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
+import com.oracle.svm.core.jdk.ProtectionDomainSupport;
+import com.oracle.svm.hosted.FeatureImpl;
 import com.oracle.svm.hosted.FeatureImpl.AfterRegistrationAccessImpl;
 import com.oracle.svm.hosted.ImageClassLoader;
 import com.oracle.svm.util.ReflectionUtil;
@@ -303,6 +306,31 @@ public void registerInvocationPlugins(Providers providers, GraphBuilderConfigura
         r.register(new ModuleEnableNativeAccessPlugin());
     }
 
+    /**
+     * For modules in the image heap, {@code Module#enableNativeAccess} is fixed at build time. We
+     * want to fold native access checks for build time modules to avoid run-time overheads as well
+     * as pulling in the error handling code unnecessarily. For example: not folding this would
+     * increase the size of hello worlds by 2x.
+     * <p>
+     * Why does this field impact the number of reachable methods so much?
+     * {@link ProtectionDomainSupport} and related {@code ProtectionDomainFeature} have a
+     * reachability handler for {@link CodeSource#getLocation}, which is not reachable by default in
+     * helloworld, but becomes reachable if {@code Module#enableNativeAccess} is not folded. The
+     * difference comes from {@code PosixNativeLibrarySupport#initializeBuiltinLibraries}, which
+     * transitively calls into {@code Module#ensureNativeAccess} and if {@code enableNativeAccess}
+     * is folded, the call to {@link CodeSource#getLocation} inside
+     * {@code Module#ensureNativeAccess} can be removed as dead code during inlining before
+     * analysis.
+     * <p>
+     * The modules for which this folding matters are {@code java.base} and
+     * {@code org.graalvm.nativeimage.builder}), both of which have {@code enableNativeAccess}
+     * already set by the time the builder starts running.
+     */
+    @Override
+    public void beforeAnalysis(BeforeAnalysisAccess access) {
+        ((FeatureImpl.BeforeAnalysisAccessImpl) access).allowStableFieldFoldingBeforeAnalysis(ModuleEnableNativeAccessPlugin.ENABLE_NATIVE_ACCESS_FIELD);
+    }
+
     /**
      * Inlines calls to {@code Module$EnableNativeAccess#isNativeAccessEnabled()} if and only if
      * {@code Module#enableNativeAccess} is true. This is ok because the field is {@code @Stable},