From 0d46a47e808dd51916aeeb0322ca7fa9ea27db2f Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Fri, 13 Dec 2024 22:55:31 +0100 Subject: [PATCH 1/3] Print more info about not materialized constants. --- .../oracle/graal/pointsto/heap/ImageHeapScanner.java | 4 ++++ .../svm/hosted/imagelayer/SVMImageLayerLoader.java | 12 ++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java index aba8d82bc991..5c62082f2c9a 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java @@ -563,6 +563,10 @@ private boolean notifyAnalysis(JavaConstant array, AnalysisType arrayType, JavaC return analysisModified; } + protected JavaConstant markReachable(JavaConstant constant, String reason) { + return markReachable(constant, new OtherReason(reason)); + } + protected JavaConstant markReachable(JavaConstant constant, ScanReason reason) { return markReachable(constant, reason, null); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java index 02ded48d5b61..b41fd5ba4cba 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java @@ -91,6 +91,7 @@ import com.oracle.svm.core.hub.DynamicHub; import com.oracle.svm.core.meta.MethodPointer; import com.oracle.svm.core.reflect.serialize.SerializationSupport; +import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.FeatureImpl; import com.oracle.svm.hosted.SVMHost; import com.oracle.svm.hosted.c.CGlobalDataFeature; @@ -1248,11 +1249,11 @@ private Object[] getReferencedValues(ImageHeapConstant parentConstant, StructLis if (delegateProcessing(constantData, values, position)) { continue; } + int finalPosition = position; values[position] = switch (constantData.which()) { case OBJECT_CONSTANT -> { int constantId = constantData.getObjectConstant().getConstantId(); boolean relink = positionsToRelink.contains(position); - int finalPosition = position; yield new AnalysisFuture<>(() -> { ensureHubInitialized(parentConstant); @@ -1279,7 +1280,14 @@ private Object[] getReferencedValues(ImageHeapConstant parentConstant, StructLis * in the base image. */ new AnalysisFuture<>(() -> { - throw AnalysisError.shouldNotReachHere("This constant was not materialized in the base image."); + String errorMessage = "Reading the value of a base layer constant which was not materialized in the base image, "; + if (parentConstant instanceof ImageHeapInstance instance) { + AnalysisField field = getFieldFromIndex(instance, finalPosition); + errorMessage += "reachable by reading field " + field + " of parent object constant: " + parentConstant; + } else { + errorMessage += "reachable by indexing at position " + finalPosition + " into parent array constant: " + parentConstant; + } + throw AnalysisError.shouldNotReachHere(errorMessage); }); case PRIMITIVE_VALUE -> { PrimitiveValue.Reader pv = constantData.getPrimitiveValue(); From aa6cf562792893a3ccd2ab81b7e229f34b2b7872 Mon Sep 17 00:00:00 2001 From: Sacha Coppey Date: Wed, 8 Jan 2025 11:39:48 +0100 Subject: [PATCH 2/3] Make all persisted constants as reachable --- .../graal/compiler/nodes/EncodedGraph.java | 16 ++++ .../oracle/graal/pointsto/ObjectScanner.java | 1 + .../graal/pointsto/api/ImageLayerWriter.java | 5 ++ .../graal/pointsto/heap/ImageHeapScanner.java | 8 +- .../graal/pointsto/meta/AnalysisMethod.java | 25 ++++++- .../imagelayer/ImageLayerBuildingSupport.java | 2 + .../svm/hosted/NativeImageGenerator.java | 8 +- .../imagelayer/SVMImageLayerLoader.java | 4 +- .../imagelayer/SVMImageLayerSnapshotUtil.java | 41 +++++++--- .../imagelayer/SVMImageLayerWriter.java | 75 +++++++------------ 10 files changed, 112 insertions(+), 73 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java index 37e4bf6baa70..1a728903eb6a 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java @@ -106,6 +106,22 @@ public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClas sourceGraph.trackNodeSourcePosition()); } + public EncodedGraph(EncodedGraph original) { + this(original.encoding, + original.startOffset, + original.objects, + original.types, + original.assumptions, + original.inlinedMethods, + original.hasUnsafeAccess, + original.trackNodeSourcePosition); + /* + * The nodeStartOffsets is only used as a cache for decoding, so it is not copied to ensure + * not having inconsistent caching information in the new EncodedGraph. + */ + this.nodeStartOffsets = null; + } + public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClass[] types, Assumptions assumptions, List inlinedMethods, boolean hasUnsafeAccess, boolean trackNodeSourcePosition) { this.encoding = encoding; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java index 56e8c9e91da9..c6ed0ef8d090 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java @@ -546,6 +546,7 @@ public static class OtherReason extends ScanReason { public static final ScanReason UNKNOWN = new OtherReason("manually created constant"); public static final ScanReason RESCAN = new OtherReason("manually triggered rescan"); public static final ScanReason HUB = new OtherReason("scanning a class constant"); + public static final ScanReason PERSISTED = new OtherReason("persisted"); final String reason; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java index 63898e56abc4..96a5132dde06 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java @@ -33,4 +33,9 @@ public class ImageLayerWriter { public void onTrackedAcrossLayer(AnalysisMethod method, Object reason) { throw AnalysisError.shouldNotReachHere("This method should not be called"); } + + @SuppressWarnings("unused") + public void persistAnalysisParsedGraph(AnalysisMethod method) { + throw AnalysisError.shouldNotReachHere("This method should not be called"); + } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java index 5c62082f2c9a..fa79361f2b89 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java @@ -497,7 +497,7 @@ private void notifyAnalysis(AnalysisField field, ImageHeapInstance receiver, Jav } private void notifyAnalysis(AnalysisField field, ImageHeapInstance receiver, JavaConstant fieldValue, ScanReason reason, Consumer onAnalysisModified) { - if (scanningObserver != null) { + if (!universe.sealed()) { /* Notify the points-to analysis of the scan. */ boolean analysisModified = doNotifyAnalysis(field, receiver, fieldValue, reason); if (analysisModified && onAnalysisModified != null) { @@ -563,11 +563,7 @@ private boolean notifyAnalysis(JavaConstant array, AnalysisType arrayType, JavaC return analysisModified; } - protected JavaConstant markReachable(JavaConstant constant, String reason) { - return markReachable(constant, new OtherReason(reason)); - } - - protected JavaConstant markReachable(JavaConstant constant, ScanReason reason) { + public JavaConstant markReachable(JavaConstant constant, ScanReason reason) { return markReachable(constant, reason, null); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 8bad6d79fe6c..97410d72c6ad 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -50,6 +50,7 @@ import com.oracle.graal.pointsto.BigBang; import com.oracle.graal.pointsto.api.ImageLayerLoader; +import com.oracle.graal.pointsto.api.ImageLayerWriter; import com.oracle.graal.pointsto.api.PointstoOptions; import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException; import com.oracle.graal.pointsto.flow.AnalysisParsedGraph; @@ -60,6 +61,7 @@ import com.oracle.graal.pointsto.infrastructure.WrappedJavaMethod; import com.oracle.graal.pointsto.reports.ReportUtils; import com.oracle.graal.pointsto.util.AnalysisError; +import com.oracle.graal.pointsto.util.AnalysisFuture; import com.oracle.graal.pointsto.util.AtomicUtils; import com.oracle.graal.pointsto.util.ConcurrentLightHashSet; import com.oracle.svm.common.meta.MultiMethod; @@ -141,6 +143,8 @@ public record Signature(String name, AnalysisType[] parameterTypes) { private final MultiMethodKey multiMethodKey; + private AnalysisFuture parsedCallBack; + /** * Map from a key to the corresponding implementation. All multi-method implementations for a * given Java method share the same map. This allows one to easily switch between different @@ -547,6 +551,20 @@ protected void notifyImplementationInvokedCallbacks() { ConcurrentLightHashSet.removeElementIf(this, implementationInvokedNotificationsUpdater, ElementNotification::isNotified); } + public void registerParsedGraphCallback(AnalysisFuture parsingCallBack) { + this.parsedCallBack = parsingCallBack; + if (getParsedGraphCacheStateObject() instanceof AnalysisParsedGraph) { + notifyParsedCallback(); + } + } + + private void notifyParsedCallback() { + if (parsedCallBack != null) { + parsedCallBack.ensureDone(); + parsedCallBack = null; + } + } + /** Get the set of all callers for this method, as inferred by the static analysis. */ public Set getCallers() { return getInvokeLocations().stream().map(location -> (AnalysisMethod) location.getMethod()).collect(Collectors.toSet()); @@ -678,12 +696,15 @@ public void onReachable(Object reason) { @Override protected void onTrackedAcrossLayers(Object reason) { AnalysisError.guarantee(!getUniverse().sealed(), "Method %s was marked as tracked after the universe was sealed", this); - getUniverse().getImageLayerWriter().onTrackedAcrossLayer(this, reason); + ImageLayerWriter imageLayerWriter = getUniverse().getImageLayerWriter(); + imageLayerWriter.onTrackedAcrossLayer(this, reason); declaringClass.registerAsTrackedAcrossLayers(reason); for (AnalysisType parameter : toParameterList()) { parameter.registerAsTrackedAcrossLayers(reason); } signature.getReturnType().registerAsTrackedAcrossLayers(reason); + + registerParsedGraphCallback(new AnalysisFuture<>(() -> imageLayerWriter.persistAnalysisParsedGraph(this))); } public void registerOverrideReachabilityNotification(MethodOverrideReachableNotification notification) { @@ -1233,6 +1254,8 @@ private AnalysisParsedGraph setGraph(BigBang bb, Stage stage, GraphCacheEntry ex boolean result = parsedGraphCacheState.compareAndSet(lockState, newEntry); AnalysisError.guarantee(result, "State transition failed"); + notifyParsedCallback(); + return (AnalysisParsedGraph) newEntry.get(stage); } catch (Throwable ex) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/ImageLayerBuildingSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/ImageLayerBuildingSupport.java index d9de1f1dbbaa..a45efbc9f044 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/ImageLayerBuildingSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/ImageLayerBuildingSupport.java @@ -78,8 +78,10 @@ protected ImageLayerBuildingSupport(boolean buildingImageLayer, boolean building */ public static void openModules() { ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.lang"); + ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.lang.reflect"); ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.util"); ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.util.concurrent"); + ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "sun.reflect.annotation"); } private static ImageLayerBuildingSupport singleton() { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index e73c3eb1ac3d..1cd6168f3f92 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -569,11 +569,6 @@ protected void doRun(Map entryPoints, JavaMainSupport j return; } - if (ImageLayerBuildingSupport.buildingSharedLayer()) { - HostedImageLayerBuildingSupport.singleton().getWriter().initializeExternalValues(); - HostedImageLayerBuildingSupport.singleton().getWriter().persistAnalysisParsedGraphs(); - } - NativeImageHeap heap; HostedMetaAccess hMetaAccess; RuntimeConfiguration runtimeConfiguration; @@ -808,6 +803,9 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De if (ImageLayerBuildingSupport.buildingImageLayer()) { ImageSingletons.lookup(LoadImageSingletonFeature.class).processRegisteredSingletons(aUniverse); } + if (ImageLayerBuildingSupport.buildingSharedLayer()) { + HostedImageLayerBuildingSupport.singleton().getWriter().initializeExternalValues(); + } } try (ReporterClosable c = ProgressReporter.singleton().printAnalysis(bb.getUniverse(), nativeLibraries.getLibraries())) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java index b41fd5ba4cba..6b501c7a1078 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java @@ -91,7 +91,6 @@ import com.oracle.svm.core.hub.DynamicHub; import com.oracle.svm.core.meta.MethodPointer; import com.oracle.svm.core.reflect.serialize.SerializationSupport; -import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.FeatureImpl; import com.oracle.svm.hosted.SVMHost; import com.oracle.svm.hosted.c.CGlobalDataFeature; @@ -824,7 +823,8 @@ private PersistedAnalysisMethod.Reader getMethodData(AnalysisMethod analysisMeth } /** - * See {@link SVMImageLayerWriter#persistAnalysisParsedGraphs()} for implementation. + * See {@link SVMImageLayerWriter#persistAnalysisParsedGraph(AnalysisMethod)} for + * implementation. */ @Override public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java index 85bc2fa7cf05..7b60b0d7d5ab 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java @@ -42,6 +42,7 @@ import org.graalvm.nativeimage.ImageSingletons; +import com.oracle.graal.pointsto.ObjectScanner; import com.oracle.graal.pointsto.heap.ImageHeapConstant; import com.oracle.graal.pointsto.heap.ImageHeapInstance; import com.oracle.graal.pointsto.heap.ImageHeapObjectArray; @@ -51,6 +52,7 @@ import com.oracle.graal.pointsto.meta.AnalysisMetaAccess; import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.graal.pointsto.meta.AnalysisType; +import com.oracle.graal.pointsto.meta.AnalysisUniverse; import com.oracle.graal.pointsto.meta.PointsToAnalysisField; import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod; import com.oracle.graal.pointsto.meta.PointsToAnalysisType; @@ -84,6 +86,7 @@ import jdk.graal.compiler.util.ObjectCopierInputStream; import jdk.graal.compiler.util.ObjectCopierOutputStream; import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod; +import jdk.vm.ci.meta.ConstantReflectionProvider; public class SVMImageLayerSnapshotUtil { public static final String FILE_NAME_PREFIX = "layer-snapshot-"; @@ -115,6 +118,8 @@ public class SVMImageLayerSnapshotUtil { protected static final Set dynamicHubRelinkedFields = Set.of(companion, name, componentType); protected static final Set dynamicHubCompanionRelinkedFields = Set.of(classInitializationInfo, superHub, arrayHub); + private static final Class sourceRoots = ReflectionUtil.lookupClass("com.oracle.svm.hosted.image.sources.SourceCache$SourceRoots"); + /** * This map stores the field indexes that should be relinked using the hosted value of a * constant from the key type. @@ -151,6 +156,10 @@ private void addSVMExternalValueFields() { continue; } + if (!shouldScanClass(clazz)) { + continue; + } + /* The ObjectCopier needs to access the static fields by reflection */ Module module = clazz.getModule(); ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, module.getName(), packageName); @@ -182,6 +191,11 @@ protected boolean shouldScanPackage(String packageName) { return true; } + private static boolean shouldScanClass(Class clazz) { + /* This class should not be scanned because it needs to be initialized after the analysis */ + return !clazz.equals(sourceRoots); + } + /** * Get all the field indexes that should be relinked using the hosted value of a constant from * the given type. @@ -207,8 +221,8 @@ private static Set getRelinkedFields(AnalysisType type, Set type return typeRelinkedFieldsSet.stream().map(metaAccess::lookupJavaField).map(AnalysisField::getPosition).collect(Collectors.toSet()); } - public SVMGraphEncoder getGraphEncoder(SVMImageLayerWriter imageLayerWriter) { - return new SVMGraphEncoder(externalValues, imageLayerWriter); + public SVMGraphEncoder getGraphEncoder() { + return new SVMGraphEncoder(externalValues); } public AbstractSVMGraphDecoder getGraphHostedToAnalysisElementsDecoder(SVMImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) { @@ -309,9 +323,9 @@ private static String getQualifiedName(AnalysisMethod method) { public static class SVMGraphEncoder extends ObjectCopier.Encoder { @SuppressWarnings("this-escape") - public SVMGraphEncoder(Map externalValues, SVMImageLayerWriter imageLayerWriter) { + public SVMGraphEncoder(Map externalValues) { super(externalValues); - addBuiltin(new ImageHeapConstantBuiltIn(imageLayerWriter, null)); + addBuiltin(new ImageHeapConstantBuiltIn(null)); addBuiltin(new AnalysisTypeBuiltIn(null)); addBuiltin(new AnalysisMethodBuiltIn(null, null)); addBuiltin(new AnalysisFieldBuiltIn(null)); @@ -333,7 +347,7 @@ public abstract static class AbstractSVMGraphDecoder extends ObjectCopier.Decode public AbstractSVMGraphDecoder(ClassLoader classLoader, SVMImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) { super(classLoader); this.imageLayerBuildingSupport = imageLayerLoader.getImageLayerBuildingSupport(); - addBuiltin(new ImageHeapConstantBuiltIn(null, imageLayerLoader)); + addBuiltin(new ImageHeapConstantBuiltIn(imageLayerLoader)); addBuiltin(new AnalysisTypeBuiltIn(imageLayerLoader)); addBuiltin(new AnalysisMethodBuiltIn(imageLayerLoader, analysisMethod)); addBuiltin(new AnalysisFieldBuiltIn(imageLayerLoader)); @@ -371,19 +385,28 @@ public SVMGraphDecoder(ClassLoader classLoader, SVMImageLayerLoader svmImageLaye } public static class ImageHeapConstantBuiltIn extends ObjectCopier.Builtin { - private final SVMImageLayerWriter imageLayerWriter; private final SVMImageLayerLoader imageLayerLoader; - protected ImageHeapConstantBuiltIn(SVMImageLayerWriter imageLayerWriter, SVMImageLayerLoader imageLayerLoader) { + protected ImageHeapConstantBuiltIn(SVMImageLayerLoader imageLayerLoader) { super(ImageHeapConstant.class, ImageHeapInstance.class, ImageHeapObjectArray.class, ImageHeapPrimitiveArray.class); - this.imageLayerWriter = imageLayerWriter; this.imageLayerLoader = imageLayerLoader; } @Override public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException { ImageHeapConstant imageHeapConstant = (ImageHeapConstant) obj; - imageLayerWriter.ensureConstantPersisted(imageHeapConstant); + + AnalysisUniverse universe = imageHeapConstant.getType().getUniverse(); + universe.getHeapScanner().markReachable(imageHeapConstant, ObjectScanner.OtherReason.PERSISTED); + + imageHeapConstant.getType().registerAsTrackedAcrossLayers(imageHeapConstant); + /* If this is a Class constant persist the corresponding type. */ + ConstantReflectionProvider constantReflection = universe.getBigbang().getConstantReflectionProvider(); + AnalysisType typeFromClassConstant = (AnalysisType) constantReflection.asJavaType(imageHeapConstant); + if (typeFromClassConstant != null) { + typeFromClassConstant.registerAsTrackedAcrossLayers(imageHeapConstant); + } + stream.writePackedUnsignedInt(ImageHeapConstant.getConstantID(imageHeapConstant)); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java index 48cd38d35cc1..b433edfd91b7 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java @@ -161,7 +161,6 @@ import jdk.graal.compiler.nodes.EncodedGraph; import jdk.graal.compiler.nodes.spi.IdentityHashCodeProvider; import jdk.graal.compiler.util.ObjectCopier; -import jdk.vm.ci.meta.ConstantReflectionProvider; import jdk.vm.ci.meta.JavaConstant; import jdk.vm.ci.meta.JavaKind; import jdk.vm.ci.meta.MethodHandleAccessProvider.IntrinsicMethod; @@ -185,13 +184,6 @@ public class SVMImageLayerWriter extends ImageLayerWriter { private final boolean useSharedLayerGraphs; private final boolean useSharedLayerStrengthenedGraphs; - private final Set constantsToPersist = ConcurrentHashMap.newKeySet(); - - public void ensureConstantPersisted(ImageHeapConstant constant) { - constantsToPersist.add(constant); - afterConstantAdded(constant); - } - private NativeImageHeap nativeImageHeap; private HostedUniverse hUniverse; @@ -326,10 +318,14 @@ public void persistAnalysisInfo() { snapshotBuilder.setStaticObjectFieldsConstantId(ImageHeapConstant.getConstantID(staticObjectFields)); // Late constant scan so all of them are known with values available (readers installed) - List constantsToScan = new ArrayList<>(constantsToPersist); + List constantsToScan = new ArrayList<>(); imageHeap.getReachableObjects().values().forEach(constantsToScan::addAll); constantsMap = HashMap.newHashMap(constantsToScan.size()); constantsToScan.forEach(c -> constantsMap.put(c, ConstantParent.NONE)); + /* + * Some child constants of reachable constants are not reachable because they are only used + * in snippets, but still need to be persisted. + */ while (!constantsToScan.isEmpty()) { List discoveredConstants = new ArrayList<>(); constantsToScan.forEach(con -> scanConstantReferencedObjects(con, discoveredConstants)); @@ -869,16 +865,6 @@ private static boolean delegateProcessing(ConstantReference.Builder builder, Obj return false; } - private void afterConstantAdded(ImageHeapConstant constant) { - constant.getType().registerAsTrackedAcrossLayers(constant); - /* If this is a Class constant persist the corresponding type. */ - ConstantReflectionProvider constantReflection = aUniverse.getBigbang().getConstantReflectionProvider(); - AnalysisType typeFromClassConstant = (AnalysisType) constantReflection.asJavaType(constant); - if (typeFromClassConstant != null) { - typeFromClassConstant.registerAsTrackedAcrossLayers(constant); - } - } - private void scanConstantReferencedObjects(ImageHeapConstant constant, Collection discoveredConstants) { if (Objects.requireNonNull(constant) instanceof ImageHeapInstance instance) { scanConstantReferencedObjects(constant, instance::getFieldValue, instance.getFieldValuesSize(), discoveredConstants); @@ -917,38 +903,20 @@ private static AnalysisMethod getRelocatableConstantMethod(MethodPointer methodP } } - public void persistAnalysisParsedGraphs() { - // Persisting graphs discovers additional types, members and constants that need persisting - Set persistedGraphMethods = new HashSet<>(); - boolean modified; - do { - modified = false; + @Override + public void persistAnalysisParsedGraph(AnalysisMethod method) { + AnalysisParsedGraph analysisParsedGraph = (AnalysisParsedGraph) method.getParsedGraphCacheStateObject(); + String name = imageLayerSnapshotUtil.getMethodDescriptor(method); + MethodGraphsInfo graphsInfo = methodsMap.get(name); + if (graphsInfo == null || graphsInfo.analysisGraphLocation == null) { /* - * GR-60503: It would be better to mark all the elements as trackedAcrossLayers before - * the end of the analysis and only iterate only once over all methods. + * A copy of the encoded graph is needed here because the nodeStartOffsets can be + * concurrently updated otherwise, which causes the ObjectCopier to fail. */ - for (AnalysisMethod method : aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers).toList()) { - if (persistedGraphMethods.add(method)) { - modified = true; - persistAnalysisParsedGraph(method); - } - } - } while (modified); - - // Note that constants are scanned late so all values are available. - } - - private void persistAnalysisParsedGraph(AnalysisMethod method) { - Object analyzedGraph = method.getParsedGraphCacheStateObject(); - if (analyzedGraph instanceof AnalysisParsedGraph analysisParsedGraph) { - String name = imageLayerSnapshotUtil.getMethodDescriptor(method); - MethodGraphsInfo graphsInfo = methodsMap.get(name); - if (graphsInfo == null || graphsInfo.analysisGraphLocation == null) { - String location = persistGraph(method, analysisParsedGraph.getEncodedGraph()); - if (location != null) { - methodsMap.compute(name, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS) - .withAnalysisGraph(location, analysisParsedGraph.isIntrinsic())); - } + String location = persistGraph(method, new EncodedGraph(analysisParsedGraph.getEncodedGraph())); + if (location != null) { + methodsMap.compute(name, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS) + .withAnalysisGraph(location, analysisParsedGraph.isIntrinsic())); } } } @@ -972,7 +940,14 @@ private String persistGraph(AnalysisMethod method, EncodedGraph analyzedGraph) { if (!useSharedLayerGraphs) { return null; } - byte[] encodedGraph = ObjectCopier.encode(imageLayerSnapshotUtil.getGraphEncoder(this), analyzedGraph); + if (Arrays.stream(analyzedGraph.getObjects()).anyMatch(o -> o instanceof AnalysisFuture)) { + /* + * GR-61103: After the AnalysisFuture in this node is handled, this check can be + * removed. + */ + return null; + } + byte[] encodedGraph = ObjectCopier.encode(imageLayerSnapshotUtil.getGraphEncoder(), analyzedGraph); if (contains(encodedGraph, LambdaUtils.LAMBDA_CLASS_NAME_SUBSTRING.getBytes(StandardCharsets.UTF_8))) { throw AnalysisError.shouldNotReachHere("The graph for the method %s contains a reference to a lambda type, which cannot be decoded: %s".formatted(method, encodedGraph)); } From 67192538a1fa899f82dad452b86d67c5deda988e Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Fri, 10 Jan 2025 15:19:13 +0100 Subject: [PATCH 3/3] remove parsedCallback field --- .../graal/pointsto/api/ImageLayerWriter.java | 3 +- .../graal/pointsto/meta/AnalysisMethod.java | 35 +++++++++---------- .../imagelayer/SVMImageLayerLoader.java | 5 +-- .../imagelayer/SVMImageLayerWriter.java | 3 +- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java index 96a5132dde06..a44c9b8646fb 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java @@ -24,6 +24,7 @@ */ package com.oracle.graal.pointsto.api; +import com.oracle.graal.pointsto.flow.AnalysisParsedGraph; import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.graal.pointsto.util.AnalysisError; @@ -35,7 +36,7 @@ public void onTrackedAcrossLayer(AnalysisMethod method, Object reason) { } @SuppressWarnings("unused") - public void persistAnalysisParsedGraph(AnalysisMethod method) { + public void persistAnalysisParsedGraph(AnalysisMethod method, AnalysisParsedGraph analysisParsedGraph) { throw AnalysisError.shouldNotReachHere("This method should not be called"); } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 97410d72c6ad..e2077ea10f02 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -39,6 +39,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.locks.ReentrantLock; @@ -61,7 +62,6 @@ import com.oracle.graal.pointsto.infrastructure.WrappedJavaMethod; import com.oracle.graal.pointsto.reports.ReportUtils; import com.oracle.graal.pointsto.util.AnalysisError; -import com.oracle.graal.pointsto.util.AnalysisFuture; import com.oracle.graal.pointsto.util.AtomicUtils; import com.oracle.graal.pointsto.util.ConcurrentLightHashSet; import com.oracle.svm.common.meta.MultiMethod; @@ -143,8 +143,6 @@ public record Signature(String name, AnalysisType[] parameterTypes) { private final MultiMethodKey multiMethodKey; - private AnalysisFuture parsedCallBack; - /** * Map from a key to the corresponding implementation. All multi-method implementations for a * given Java method share the same map. This allows one to easily switch between different @@ -178,6 +176,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) { private final boolean enableReachableInCurrentLayer; private final AtomicReference parsedGraphCacheState = new AtomicReference<>(GraphCacheEntry.UNPARSED); + private final AtomicBoolean trackedGraphPersisted = new AtomicBoolean(false); private EncodedGraph analyzedGraph; @@ -551,17 +550,10 @@ protected void notifyImplementationInvokedCallbacks() { ConcurrentLightHashSet.removeElementIf(this, implementationInvokedNotificationsUpdater, ElementNotification::isNotified); } - public void registerParsedGraphCallback(AnalysisFuture parsingCallBack) { - this.parsedCallBack = parsingCallBack; - if (getParsedGraphCacheStateObject() instanceof AnalysisParsedGraph) { - notifyParsedCallback(); - } - } - - private void notifyParsedCallback() { - if (parsedCallBack != null) { - parsedCallBack.ensureDone(); - parsedCallBack = null; + private void persistTrackedGraph(AnalysisParsedGraph graph) { + if (isTrackedAcrossLayers() && trackedGraphPersisted.compareAndSet(false, true)) { + ImageLayerWriter imageLayerWriter = getUniverse().getImageLayerWriter(); + imageLayerWriter.persistAnalysisParsedGraph(this, graph); } } @@ -696,15 +688,16 @@ public void onReachable(Object reason) { @Override protected void onTrackedAcrossLayers(Object reason) { AnalysisError.guarantee(!getUniverse().sealed(), "Method %s was marked as tracked after the universe was sealed", this); - ImageLayerWriter imageLayerWriter = getUniverse().getImageLayerWriter(); - imageLayerWriter.onTrackedAcrossLayer(this, reason); + getUniverse().getImageLayerWriter().onTrackedAcrossLayer(this, reason); declaringClass.registerAsTrackedAcrossLayers(reason); for (AnalysisType parameter : toParameterList()) { parameter.registerAsTrackedAcrossLayers(reason); } signature.getReturnType().registerAsTrackedAcrossLayers(reason); - registerParsedGraphCallback(new AnalysisFuture<>(() -> imageLayerWriter.persistAnalysisParsedGraph(this))); + if (getParsedGraphCacheStateObject() instanceof AnalysisParsedGraph analysisParsedGraph) { + persistTrackedGraph(analysisParsedGraph); + } } public void registerOverrideReachabilityNotification(MethodOverrideReachableNotification notification) { @@ -1254,9 +1247,13 @@ private AnalysisParsedGraph setGraph(BigBang bb, Stage stage, GraphCacheEntry ex boolean result = parsedGraphCacheState.compareAndSet(lockState, newEntry); AnalysisError.guarantee(result, "State transition failed"); - notifyParsedCallback(); + AnalysisParsedGraph analysisParsedGraph = (AnalysisParsedGraph) newEntry.get(stage); + + if (stage == Stage.finalStage()) { + persistTrackedGraph(analysisParsedGraph); + } - return (AnalysisParsedGraph) newEntry.get(stage); + return analysisParsedGraph; } catch (Throwable ex) { parsedGraphCacheState.set(GraphCacheEntry.createParsingError(stage, expectedValue, ex)); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java index 6b501c7a1078..ccd8eb14f165 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java @@ -823,8 +823,9 @@ private PersistedAnalysisMethod.Reader getMethodData(AnalysisMethod analysisMeth } /** - * See {@link SVMImageLayerWriter#persistAnalysisParsedGraph(AnalysisMethod)} for - * implementation. + * See + * {@link SVMImageLayerWriter#persistAnalysisParsedGraph(AnalysisMethod, AnalysisParsedGraph)} + * for implementation. */ @Override public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java index b433edfd91b7..fd6d4d46ad9c 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java @@ -904,8 +904,7 @@ private static AnalysisMethod getRelocatableConstantMethod(MethodPointer methodP } @Override - public void persistAnalysisParsedGraph(AnalysisMethod method) { - AnalysisParsedGraph analysisParsedGraph = (AnalysisParsedGraph) method.getParsedGraphCacheStateObject(); + public void persistAnalysisParsedGraph(AnalysisMethod method, AnalysisParsedGraph analysisParsedGraph) { String name = imageLayerSnapshotUtil.getMethodDescriptor(method); MethodGraphsInfo graphsInfo = methodsMap.get(name); if (graphsInfo == null || graphsInfo.analysisGraphLocation == null) {