From 398a5a09163e18c94ec711837b68fdb40cce8f49 Mon Sep 17 00:00:00 2001 From: Sacha Coppey Date: Mon, 25 Nov 2024 14:34:57 +0100 Subject: [PATCH 1/2] Improve trackAcrossLayers --- .../graal/pointsto/heap/ImageLayerLoader.java | 4 +- .../pointsto/heap/ImageLayerSnapshotUtil.java | 51 ++++----- .../graal/pointsto/heap/ImageLayerWriter.java | 101 ++++++------------ .../pointsto/heap/ImageLayerWriterHelper.java | 2 +- .../graal/pointsto/meta/AnalysisElement.java | 25 ++++- .../graal/pointsto/meta/AnalysisField.java | 21 +--- .../graal/pointsto/meta/AnalysisMethod.java | 36 +++---- .../graal/pointsto/meta/AnalysisType.java | 43 ++++---- .../svm/hosted/NativeImageGenerator.java | 13 +-- .../heap/SVMImageLayerSnapshotUtil.java | 1 - .../svm/hosted/heap/SVMImageLayerWriter.java | 7 +- .../heap/SVMImageLayerWriterHelper.java | 6 +- .../LayeredDispatchTableSupport.java | 20 +++- 13 files changed, 148 insertions(+), 182 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java index d89d6b431fed..1055b50df475 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java @@ -588,8 +588,8 @@ public void initializeBaseLayerMethod(AnalysisMethod analysisMethod) { /** * Currently we save analysis parsed graphs for methods considered - * {@link AnalysisMethod#isReachable}. See {@link ImageLayerWriter#persistMethodGraphs} for - * implementation. + * {@link AnalysisMethod#isTrackedAcrossLayers()}. See + * {@link ImageLayerWriter#persistAnalysisParsedGraph} for implementation. */ public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) { return getMethodData(analysisMethod).hasAnalysisGraphLocation(); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java index 770ff202c770..a770e6607931 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java @@ -62,6 +62,8 @@ public class ImageLayerSnapshotUtil { public static final int UNDEFINED_CONSTANT_ID = -1; public static final int UNDEFINED_FIELD_INDEX = -1; + private static final String TRACKED_REASON = "reachable from a graph"; + protected final List externalValueFields; /** This needs to be initialized after analysis, as some fields are not available before. */ protected Map externalValues; @@ -140,10 +142,10 @@ public static class GraphEncoder extends ObjectCopier.Encoder { public GraphEncoder(Map externalValues, ImageLayerWriter imageLayerWriter) { super(externalValues); addBuiltin(new ImageHeapConstantBuiltIn(imageLayerWriter, null)); - addBuiltin(new AnalysisTypeBuiltIn(imageLayerWriter, null)); - addBuiltin(new AnalysisMethodBuiltIn(imageLayerWriter, null, null)); - addBuiltin(new AnalysisFieldBuiltIn(imageLayerWriter, null)); - addBuiltin(new FieldLocationIdentityBuiltIn(imageLayerWriter, null)); + addBuiltin(new AnalysisTypeBuiltIn(null)); + addBuiltin(new AnalysisMethodBuiltIn(null, null)); + addBuiltin(new AnalysisFieldBuiltIn(null)); + addBuiltin(new FieldLocationIdentityBuiltIn(null)); } } @@ -155,10 +157,10 @@ public GraphDecoder(ClassLoader classLoader, ImageLayerLoader imageLayerLoader, super(classLoader); this.imageLayerLoader = imageLayerLoader; addBuiltin(new ImageHeapConstantBuiltIn(null, imageLayerLoader)); - addBuiltin(new AnalysisTypeBuiltIn(null, imageLayerLoader)); - addBuiltin(new AnalysisMethodBuiltIn(null, imageLayerLoader, analysisMethod)); - addBuiltin(new AnalysisFieldBuiltIn(null, imageLayerLoader)); - addBuiltin(new FieldLocationIdentityBuiltIn(null, imageLayerLoader)); + addBuiltin(new AnalysisTypeBuiltIn(imageLayerLoader)); + addBuiltin(new AnalysisMethodBuiltIn(imageLayerLoader, analysisMethod)); + addBuiltin(new AnalysisFieldBuiltIn(imageLayerLoader)); + addBuiltin(new FieldLocationIdentityBuiltIn(imageLayerLoader)); } @Override @@ -192,19 +194,17 @@ protected Object decode(ObjectCopier.Decoder decoder, Class concreteType, Obj } public static class AnalysisTypeBuiltIn extends ObjectCopier.Builtin { - private final ImageLayerWriter imageLayerWriter; private final ImageLayerLoader imageLayerLoader; - protected AnalysisTypeBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoader imageLayerLoader) { + protected AnalysisTypeBuiltIn(ImageLayerLoader imageLayerLoader) { super(AnalysisType.class, PointsToAnalysisType.class); - this.imageLayerWriter = imageLayerWriter; this.imageLayerLoader = imageLayerLoader; } @Override public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException { AnalysisType type = (AnalysisType) obj; - imageLayerWriter.ensureTypePersisted(type); + type.registerAsTrackedAcrossLayers(TRACKED_REASON); stream.writePackedUnsignedInt(type.getId()); } @@ -216,13 +216,11 @@ protected Object decode(ObjectCopier.Decoder decoder, Class concreteType, Obj } public static class AnalysisMethodBuiltIn extends ObjectCopier.Builtin { - private final ImageLayerWriter imageLayerWriter; private final ImageLayerLoader imageLayerLoader; private final AnalysisMethod analysisMethod; - protected AnalysisMethodBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod) { + protected AnalysisMethodBuiltIn(ImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod) { super(AnalysisMethod.class, PointsToAnalysisMethod.class); - this.imageLayerWriter = imageLayerWriter; this.imageLayerLoader = imageLayerLoader; this.analysisMethod = analysisMethod; } @@ -230,12 +228,7 @@ protected AnalysisMethodBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoa @Override public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException { AnalysisMethod method = (AnalysisMethod) obj; - AnalysisType declaringClass = method.getDeclaringClass(); - imageLayerWriter.ensureMethodPersisted(method); - for (AnalysisType parameter : method.toParameterList()) { - imageLayerWriter.ensureTypePersisted(parameter); - } - imageLayerWriter.ensureTypePersisted(declaringClass); + method.registerAsTrackedAcrossLayers(TRACKED_REASON); stream.writePackedUnsignedInt(method.getId()); } @@ -250,19 +243,17 @@ protected Object decode(ObjectCopier.Decoder decoder, Class concreteType, Obj } public static class AnalysisFieldBuiltIn extends ObjectCopier.Builtin { - private final ImageLayerWriter imageLayerWriter; private final ImageLayerLoader imageLayerLoader; - protected AnalysisFieldBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoader imageLayerLoader) { + protected AnalysisFieldBuiltIn(ImageLayerLoader imageLayerLoader) { super(AnalysisField.class, PointsToAnalysisField.class); - this.imageLayerWriter = imageLayerWriter; this.imageLayerLoader = imageLayerLoader; } @Override public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException { AnalysisField field = (AnalysisField) obj; - int id = encodeField(field, imageLayerWriter); + int id = encodeField(field); stream.writePackedUnsignedInt(id); } @@ -274,12 +265,10 @@ protected Object decode(ObjectCopier.Decoder decoder, Class concreteType, Obj } public static class FieldLocationIdentityBuiltIn extends ObjectCopier.Builtin { - private final ImageLayerWriter imageLayerWriter; private final ImageLayerLoader imageLayerLoader; - protected FieldLocationIdentityBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoader imageLayerLoader) { + protected FieldLocationIdentityBuiltIn(ImageLayerLoader imageLayerLoader) { super(FieldLocationIdentity.class); - this.imageLayerWriter = imageLayerWriter; this.imageLayerLoader = imageLayerLoader; } @@ -287,7 +276,7 @@ protected FieldLocationIdentityBuiltIn(ImageLayerWriter imageLayerWriter, ImageL public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException { FieldLocationIdentity fieldLocationIdentity = (FieldLocationIdentity) obj; AnalysisField field = (AnalysisField) fieldLocationIdentity.getField(); - int id = encodeField(field, imageLayerWriter); + int id = encodeField(field); stream.writePackedUnsignedInt(id); } @@ -298,8 +287,8 @@ protected Object decode(ObjectCopier.Decoder decoder, Class concreteType, Obj } } - private static int encodeField(AnalysisField field, ImageLayerWriter imageLayerWriter) { - imageLayerWriter.ensureFieldPersisted(field); + private static int encodeField(AnalysisField field) { + field.registerAsTrackedAcrossLayers(TRACKED_REASON); return field.getId(); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java index 7887d4bf8c57..45de7b9541e0 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java @@ -119,38 +119,9 @@ public class ImageLayerWriter { private final boolean useSharedLayerGraphs; private final boolean useSharedLayerStrengthenedGraphs; - /* - * Types, members and constants to persist even when they are not considered reachable by the - * analysis, or referenced from the image heap. Typically, these elements would be reachable - * from a persisted graph. - */ - private boolean sealed = false; - private final Set typesToPersist = ConcurrentHashMap.newKeySet(); - private final Set methodsToPersist = ConcurrentHashMap.newKeySet(); - private final Set fieldsToPersist = ConcurrentHashMap.newKeySet(); private final Set constantsToPersist = ConcurrentHashMap.newKeySet(); - public void ensureTypePersisted(AnalysisType type) { - assert !sealed; - if (typesToPersist.add(type)) { - afterTypeAdded(type); - } - } - - public void ensureMethodPersisted(AnalysisMethod method) { - assert !sealed; - if (methodsToPersist.add(method)) { - afterMethodAdded(method); - } - } - - public void ensureFieldPersisted(AnalysisField field) { - assert !sealed; - fieldsToPersist.add(field); - } - public void ensureConstantPersisted(ImageHeapConstant constant) { - assert !sealed; constantsToPersist.add(constant); afterConstantAdded(constant); } @@ -162,7 +133,8 @@ protected record ConstantParent(int constantId, int index) { private record FileInfo(Path layerFilePath, String fileName, String suffix) { } - protected record MethodGraphsInfo(String analysisGraphLocation, boolean analysisGraphIsIntrinsic, String strengthenedGraphLocation) { + protected record MethodGraphsInfo(String analysisGraphLocation, boolean analysisGraphIsIntrinsic, + String strengthenedGraphLocation) { static final MethodGraphsInfo NO_GRAPHS = new MethodGraphsInfo(null, false, null); @@ -252,6 +224,11 @@ public void setAnalysisUniverse(AnalysisUniverse aUniverse) { this.aUniverse = aUniverse; } + @SuppressWarnings("unused") + public void onTrackedAcrossLayer(AnalysisMethod method, Object reason) { + imageLayerWriterHelper.onTrackedAcrossLayer(method, reason); + } + public void dumpFiles() { graphsOutput.finish(); @@ -291,6 +268,10 @@ public void persistAnalysisInfo() { snapshotBuilder.setNextFieldId(aUniverse.getNextFieldId()); snapshotBuilder.setNextConstantId(ImageHeapConstant.getCurrentId()); + List typesToPersist = aUniverse.getTypes().stream().filter(AnalysisType::isTrackedAcrossLayers).toList(); + List methodsToPersist = aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers).toList(); + List fieldsToPersist = aUniverse.getFields().stream().filter(AnalysisField::isTrackedAcrossLayers).toList(); + initSortedList(snapshotBuilder::initTypes, typesToPersist, Comparator.comparingInt(AnalysisType::getId), this::persistType); initSortedList(snapshotBuilder::initMethods, methodsToPersist, Comparator.comparingInt(AnalysisMethod::getId), this::persistMethod); initSortedList(snapshotBuilder::initFields, fieldsToPersist, Comparator.comparingInt(AnalysisField::getId), this::persistField); @@ -345,7 +326,7 @@ protected void persistHook() { } public boolean isTypePersisted(AnalysisType type) { - return typesToPersist.contains(type); + return type.isTrackedAcrossLayers(); } private void persistType(AnalysisType type, Supplier builderSupplier) { @@ -373,12 +354,6 @@ protected void persistType(AnalysisType type, String typeDescriptor, PersistedAn if (enclosingType != null) { builder.setEnclosingTypeId(enclosingType.getId()); } - } catch (AnalysisError.TypeNotFoundError e) { - /* - * GR-59571: The enclosing type is not automatically created when the inner type is - * created. If the enclosing type is missing, it is ignored for now. This try/catch - * block could be removed after the trackAcrossLayers is fully implemented. - */ } catch (InternalError | TypeNotPresentException | LinkageError e) { /* Ignore missing type errors. */ } @@ -399,6 +374,8 @@ protected void persistType(AnalysisType type, String typeDescriptor, PersistedAn builder.setIsReachable(type.isReachable()); imageLayerWriterHelper.persistType(type, builder); + + afterTypeAdded(type); } protected static void initInts(IntFunction builderSupplier, IntStream ids) { @@ -417,31 +394,17 @@ protected static void initStringList(IntFunction builderSuppli } } + @SuppressWarnings("unused") protected void afterTypeAdded(AnalysisType type) { - /* - * Some persisted types are not reachable. In this case, the super class and interfaces have - * to be persisted manually as well. - */ - if (type.getSuperclass() != null) { - ensureTypePersisted(type.getSuperclass()); - } - for (AnalysisType iface : type.getInterfaces()) { - ensureTypePersisted(iface); - } - } - - protected void afterMethodAdded(AnalysisMethod method) { - ensureTypePersisted(method.getSignature().getReturnType()); - imageLayerWriterHelper.afterMethodAdded(method); } private void afterConstantAdded(ImageHeapConstant constant) { - ensureTypePersisted(constant.getType()); + 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) { - ensureTypePersisted(typeFromClassConstant); + typeFromClassConstant.registerAsTrackedAcrossLayers(constant); } } @@ -535,31 +498,25 @@ public boolean isMethodPersisted(AnalysisMethod method) { return methodsMap.containsKey(name); } - public void persistMethodGraphs() { - assert aUniverse.sealed(); - - aUniverse.getTypes().stream().filter(AnalysisType::isTrackedAcrossLayers) - .forEach(this::ensureTypePersisted); - - aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers) - .forEach(this::ensureMethodPersisted); - - aUniverse.getFields().stream().filter(AnalysisField::isTrackedAcrossLayers) - .forEach(this::ensureFieldPersisted); - + public void persistAnalysisParsedGraphs() { // Persisting graphs discovers additional types, members and constants that need persisting Set persistedGraphMethods = new HashSet<>(); + boolean modified; do { - for (AnalysisMethod method : methodsToPersist) { + modified = false; + /* + * 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. + */ + for (AnalysisMethod method : aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers).toList()) { if (persistedGraphMethods.add(method)) { + modified = true; persistAnalysisParsedGraph(method); } } - } while (!persistedGraphMethods.equals(methodsToPersist)); + } while (modified); // Note that constants are scanned late so all values are available. - - sealed = true; } private void persistAnalysisParsedGraph(AnalysisMethod method) { @@ -647,7 +604,9 @@ protected void persistField(AnalysisField field, PersistedAnalysisField.Builder protected void persistConstant(ImageHeapConstant imageHeapConstant, ConstantParent parent, PersistedConstant.Builder builder, Set constantsToRelink) { int id = getConstantId(imageHeapConstant); builder.setId(id); - builder.setTypeId(imageHeapConstant.getType().getId()); + AnalysisType type = imageHeapConstant.getType(); + AnalysisError.guarantee(type.isTrackedAcrossLayers(), "Type %s from constant %s should have been marked as trackedAcrossLayers, but was not", type, imageHeapConstant); + builder.setTypeId(type.getId()); IdentityHashCodeProvider identityHashCodeProvider = (IdentityHashCodeProvider) aUniverse.getBigbang().getConstantReflectionProvider(); int identityHashCode = identityHashCodeProvider.identityHashCode(imageHeapConstant); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriterHelper.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriterHelper.java index a5088f451984..cc26c326c49d 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriterHelper.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriterHelper.java @@ -47,6 +47,6 @@ protected void persistMethod(AnalysisMethod method, PersistedAnalysisMethod.Buil } @SuppressWarnings("unused") - protected void afterMethodAdded(AnalysisMethod method) { + protected void onTrackedAcrossLayer(AnalysisMethod method, Object reason) { } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java index f1b0a7fe4cd2..0bbebc55503b 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java @@ -38,6 +38,7 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; +import com.oracle.graal.pointsto.util.AtomicUtils; import org.graalvm.nativeimage.hosted.Feature.DuringAnalysisAccess; import com.oracle.graal.pointsto.BigBang; @@ -57,6 +58,18 @@ public abstract class AnalysisElement implements AnnotatedElement { + protected static final AtomicReferenceFieldUpdater trackAcrossLayersUpdater = AtomicReferenceFieldUpdater + .newUpdater(AnalysisElement.class, Object.class, "trackAcrossLayers"); + /** + * See {@link AnalysisElement#isTrackedAcrossLayers} for explanation. + */ + @SuppressWarnings("unused") private volatile Object trackAcrossLayers; + protected final boolean enableTrackAcrossLayers; + + protected AnalysisElement(boolean enableTrackAcrossLayers) { + this.enableTrackAcrossLayers = enableTrackAcrossLayers; + } + public abstract AnnotatedElement getWrapped(); protected abstract AnalysisUniverse getUniverse(); @@ -114,10 +127,20 @@ protected void notifyReachabilityCallbacks(AnalysisUniverse universe, List onTrackedAcrossLayers(reason)); + } + } + /** * Indicates we need this information to be saved in the layer archive file. */ - public abstract boolean isTrackedAcrossLayers(); + public boolean isTrackedAcrossLayers() { + return AtomicUtils.isSet(this, trackAcrossLayersUpdater); + } + + protected abstract void onTrackedAcrossLayers(Object reason); /** Return true if reachability handlers should be executed for this element. */ public boolean isTriggered() { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java index 7a8718ba80a8..616fd47c55c4 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java @@ -64,10 +64,6 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa private static final AtomicReferenceFieldUpdater isUnsafeAccessedUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisField.class, Object.class, "isUnsafeAccessed"); - - private static final AtomicReferenceFieldUpdater trackAcrossLayersUpdater = AtomicReferenceFieldUpdater - .newUpdater(AnalysisField.class, Object.class, "trackAcrossLayers"); - private final int id; /** Marks a field loaded from a base layer. */ private final boolean isInBaseLayer; @@ -93,12 +89,6 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa @SuppressWarnings("unused") private volatile Object isFolded; @SuppressWarnings("unused") private volatile Object isUnsafeAccessed; - /** - * See {@link AnalysisElement#isTrackedAcrossLayers} for explanation. - */ - @SuppressWarnings("unused") private volatile Object trackAcrossLayers; - private final boolean enableTrackAcrossLayers; - private ConcurrentMap readBy; private ConcurrentMap writtenBy; @@ -117,6 +107,7 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa @SuppressWarnings("this-escape") public AnalysisField(AnalysisUniverse universe, ResolvedJavaField wrappedField) { + super(universe.hostVM.enableTrackAcrossLayers()); assert !wrappedField.isInternal() : wrappedField; this.position = -1; @@ -159,8 +150,6 @@ public AnalysisField(AnalysisUniverse universe, ResolvedJavaField wrappedField) id = universe.computeNextFieldId(); isInBaseLayer = false; } - - this.enableTrackAcrossLayers = universe.hostVM.enableTrackAcrossLayers(); } @Override @@ -383,15 +372,13 @@ public boolean isReachable() { @Override public void onReachable(Object reason) { - if (enableTrackAcrossLayers) { - AtomicUtils.atomicSet(this, reason, trackAcrossLayersUpdater); - } + registerAsTrackedAcrossLayers(reason); notifyReachabilityCallbacks(declaringClass.getUniverse(), new ArrayList<>()); } @Override - public boolean isTrackedAcrossLayers() { - return AtomicUtils.isSet(this, trackAcrossLayersUpdater); + protected void onTrackedAcrossLayers(Object reason) { + AnalysisError.guarantee(!getUniverse().sealed(), "Field %s was marked as tracked after the universe was sealed", this); } public Object getFieldValueInterceptor() { 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 dc1b43c5bad2..4e2fe934a00b 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 @@ -112,9 +112,6 @@ public abstract class AnalysisMethod extends AnalysisElement implements WrappedJ static final AtomicReferenceFieldUpdater allImplementationsUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisMethod.class, Object.class, "allImplementations"); - private static final AtomicReferenceFieldUpdater trackAcrossLayersUpdater = AtomicReferenceFieldUpdater - .newUpdater(AnalysisMethod.class, Object.class, "trackAcrossLayers"); - public record Signature(String name, AnalysisType[] parameterTypes) { } @@ -184,12 +181,6 @@ public record Signature(String name, AnalysisType[] parameterTypes) { */ @SuppressWarnings("unused") private volatile Object allImplementations; - /** - * See {@link AnalysisElement#isTrackedAcrossLayers} for explanation. - */ - @SuppressWarnings("unused") private volatile Object trackAcrossLayers; - private final boolean enableTrackAcrossLayers; - /** * Indicates that this method has opaque return. This is necessary when there are control flows * present which cannot be tracked by analysis, which happens for continuation support. @@ -201,6 +192,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) { @SuppressWarnings({"this-escape", "unchecked"}) protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped, MultiMethodKey multiMethodKey, Map multiMethodMap) { + super(universe.hostVM.enableTrackAcrossLayers()); this.wrapped = wrapped; declaringClass = universe.lookup(wrapped.getDeclaringClass()); @@ -274,12 +266,11 @@ protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped, startTrackInvocations(); } parsingContextMaxDepth = PointstoOptions.ParsingContextMaxDepth.getValue(declaringClass.universe.hostVM.options()); - - this.enableTrackAcrossLayers = universe.hostVM.enableTrackAcrossLayers(); } @SuppressWarnings("this-escape") protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey) { + super(original.enableTrackAcrossLayers); wrapped = original.wrapped; id = original.id; isInBaseLayer = original.isInBaseLayer; @@ -303,8 +294,6 @@ protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey) if (PointstoOptions.TrackAccessChain.getValue(declaringClass.universe.hostVM().options())) { startTrackInvocations(); } - - this.enableTrackAcrossLayers = original.enableTrackAcrossLayers; } private static String createName(ResolvedJavaMethod wrapped, MultiMethodKey multiMethodKey) { @@ -492,6 +481,7 @@ public void registerAsEntryPoint(Object newEntryPointData) { public boolean registerAsInvoked(Object reason) { assert isValidReason(reason) : "Registering a method as invoked needs to provide a valid reason, found: " + reason; + registerAsTrackedAcrossLayers(reason); return AtomicUtils.atomicSet(this, reason, isInvokedUpdater); } @@ -649,11 +639,6 @@ public boolean isReachable() { return isImplementationInvoked() || isInlined(); } - @Override - public boolean isTrackedAcrossLayers() { - return AtomicUtils.isSet(this, trackAcrossLayersUpdater); - } - @Override public boolean isTriggered() { if (isReachable()) { @@ -669,12 +654,21 @@ public void onImplementationInvoked(Object reason) { @Override public void onReachable(Object reason) { - if (enableTrackAcrossLayers) { - AtomicUtils.atomicSet(this, reason, trackAcrossLayersUpdater); - } + registerAsTrackedAcrossLayers(reason); notifyReachabilityCallbacks(declaringClass.getUniverse(), new ArrayList<>()); } + @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); + declaringClass.registerAsTrackedAcrossLayers(reason); + for (AnalysisType parameter : toParameterList()) { + parameter.registerAsTrackedAcrossLayers(reason); + } + signature.getReturnType().registerAsTrackedAcrossLayers(reason); + } + public void registerOverrideReachabilityNotification(MethodOverrideReachableNotification notification) { getUniverse().registerOverrideReachabilityNotification(this, notification); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 55d8acd023f9..c124c9439cad 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -108,9 +108,6 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav static final AtomicReferenceFieldUpdater overrideableMethodsUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisType.class, Object.class, "overrideableMethods"); - private static final AtomicReferenceFieldUpdater trackAcrossLayersUpdater = AtomicReferenceFieldUpdater - .newUpdater(AnalysisType.class, Object.class, "trackAcrossLayers"); - protected final AnalysisUniverse universe; private final ResolvedJavaType wrapped; private final String qualifiedName; @@ -221,14 +218,9 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav */ @SuppressWarnings("unused") private volatile Object overrideableMethods; - /** - * See {@link AnalysisElement#isTrackedAcrossLayers} for explanation. - */ - @SuppressWarnings("unused") private volatile Object trackAcrossLayers; - private final boolean enableTrackAcrossLayers; - @SuppressWarnings("this-escape") public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKind storageKind, AnalysisType objectType, AnalysisType cloneableType) { + super(universe.hostVM.enableTrackAcrossLayers()); this.universe = universe; this.wrapped = javaType; qualifiedName = wrapped.toJavaName(true); @@ -352,8 +344,6 @@ public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKi AnalysisError.guarantee(universe.getHeapScanner() != null, "Heap scanner is not available."); return universe.getHeapScanner().computeTypeData(this); }); - - this.enableTrackAcrossLayers = universe.hostVM.enableTrackAcrossLayers(); } private AnalysisType[] convertTypes(ResolvedJavaType[] originalTypes) { @@ -611,9 +601,7 @@ public boolean registerAsReachable(Object reason) { @Override protected void onReachable(Object reason) { - if (enableTrackAcrossLayers) { - AtomicUtils.atomicSet(this, reason, trackAcrossLayersUpdater); - } + registerAsTrackedAcrossLayers(reason); List> futures = new ArrayList<>(); notifyReachabilityCallbacks(universe, futures); @@ -651,6 +639,28 @@ protected void onReachable(Object reason) { ensureOnTypeReachableTaskDone(); } + @Override + protected void onTrackedAcrossLayers(Object reason) { + AnalysisError.guarantee(!getUniverse().sealed(), "Type %s was marked as tracked after the universe was sealed", this); + if (superClass != null) { + superClass.registerAsTrackedAcrossLayers(reason); + } + for (AnalysisType inter : interfaces) { + inter.registerAsTrackedAcrossLayers(reason); + } + try { + AnalysisType enclosingType = getEnclosingType(); + if (enclosingType != null) { + enclosingType.registerAsTrackedAcrossLayers(reason); + } + } catch (InternalError | TypeNotPresentException | LinkageError e) { + /* + * Ignore missing type errors. The build process should not fail if the incomplete type + * is not reached through other paths. + */ + } + } + /** Prepare information that {@link AnalysisMethod#collectMethodImplementations} needs. */ private void prepareMethodImplementations(AnalysisType superType) { ConcurrentLightHashSet.forEach(superType, overrideableMethodsUpdater, (AnalysisMethod method) -> { @@ -875,11 +885,6 @@ public Object getReachableReason() { return isReachable; } - @Override - public boolean isTrackedAcrossLayers() { - return AtomicUtils.isSet(this, trackAcrossLayersUpdater); - } - /** * The kind of the field in memory (in contrast to {@link #getJavaKind()}, which is the kind of * the field on the Java type system level). For example {@link WordBase word types} have a 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 89c9e5658ec1..4790067887f0 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 @@ -570,6 +570,11 @@ 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; @@ -588,10 +593,6 @@ protected void doRun(Map entryPoints, JavaMainSupport j BeforeUniverseBuildingAccessImpl beforeUniverseBuildingConfig = new BeforeUniverseBuildingAccessImpl(featureHandler, loader, debug, hMetaAccess); featureHandler.forEachFeature(feature -> feature.beforeUniverseBuilding(beforeUniverseBuildingConfig)); - if (ImageLayerBuildingSupport.buildingSharedLayer()) { - HostedImageLayerBuildingSupport.singleton().getWriter().initializeExternalValues(); - } - new UniverseBuilder(aUniverse, bb.getMetaAccess(), hUniverse, hMetaAccess, HostedConfiguration.instance().createStrengthenGraphs(bb, hUniverse), bb.getUnsupportedFeatures()).build(debug); @@ -627,10 +628,6 @@ protected void doRun(Map entryPoints, JavaMainSupport j recordRestrictHeapAccessCallees(aUniverse.getMethods()); - if (ImageLayerBuildingSupport.buildingSharedLayer()) { - HostedImageLayerBuildingSupport.singleton().getWriter().persistMethodGraphs(); - } - /* * After this point, all TypeFlow (and therefore also TypeState) objects are * unreachable and can be garbage collected. This is important to keep the overall diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerSnapshotUtil.java index 4e9a18fc89f3..3ffcead2c4b7 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerSnapshotUtil.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerSnapshotUtil.java @@ -238,7 +238,6 @@ public static class SVMGraphEncoder extends GraphEncoder { @SuppressWarnings("this-escape") public SVMGraphEncoder(Map externalValues, ImageLayerWriter imageLayerWriter) { super(externalValues, imageLayerWriter); - addBuiltin(new HostedTypeBuiltIn(null)); addBuiltin(new HostedMethodBuiltIn(null)); addBuiltin(new HostedOptionValuesBuiltIn()); addBuiltin(new HostedSnippetReflectionProviderBuiltIn(null)); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java index b248885cdebb..7db4a4bf90d9 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java @@ -247,7 +247,7 @@ protected void scanConstantReferencedObjects(ImageHeapConstant constant, Object[ if (referencedObjects != null) { for (Object obj : referencedObjects) { if (obj instanceof MethodPointer mp) { - ensureMethodPersisted(getRelocatableConstantMethod(mp)); + getRelocatableConstantMethod(mp).registerAsTrackedAcrossLayers("In method pointer"); } } } @@ -389,7 +389,10 @@ private static void writeImageSingletonKeyStore(ImageSingletonObject.Builder obj case String[] strl -> { TextList.Builder strlb = b.getValue().initStrl(strl.length); for (int j = 0; j < strl.length; j++) { - strlb.set(j, new Text.Reader(strl[j])); + String s = strl[j]; + if (s != null) { + strlb.set(j, new Text.Reader(s)); + } } } case boolean[] zl -> { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java index 4c4b63643d44..9f02e89b8221 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java @@ -103,12 +103,12 @@ protected void persistMethod(AnalysisMethod method, PersistedAnalysisMethod.Buil } @Override - protected void afterMethodAdded(AnalysisMethod method) { + public void onTrackedAcrossLayer(AnalysisMethod method, Object reason) { if (method.wrapped instanceof FactoryMethod factoryMethod) { AnalysisMethod targetConstructor = method.getUniverse().lookup(factoryMethod.getTargetConstructor()); - imageLayerWriter.ensureMethodPersisted(targetConstructor); + targetConstructor.registerAsTrackedAcrossLayers(reason); } - super.afterMethodAdded(method); + super.onTrackedAcrossLayer(method, reason); } private static void persistWrappedMember(WrappedMethod.WrappedMember.Builder b, Executable member) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java index b573ab417c9f..91e26d4ec619 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java @@ -228,7 +228,7 @@ private void injectPriorLayerInfo(HostedType type, HostedDispatchTable dispatchT var priorInfo = priorDispatchTables.get(type.getWrapped().getId()); if (priorInfo != null) { compareTypeInfo(dispatchTable, priorInfo); - dispatchTable.status = HubStatus.INSTALLED_PRIOR_LAYER; + dispatchTable.status = priorInfo.installed ? HubStatus.INSTALLED_PRIOR_LAYER : HubStatus.COMPUTED_PRIOR_LAYER; for (int i = 0; i < dispatchTable.slots.length; i++) { HostedDispatchSlot slot = dispatchTable.slots[i]; PriorDispatchSlot priorSlot = priorInfo.slots[i]; @@ -326,7 +326,7 @@ public void registerWrittenDynamicHub(DynamicHub hub, AnalysisUniverse aUniverse var dispatchTable = typeToDispatchTable.get(hType); // upgrade status to being installed in current layer - assert dispatchTable.status == HubStatus.DISPATCH_INFO_CALCULATED : dispatchTable; + assert dispatchTable.status == HubStatus.DISPATCH_INFO_CALCULATED || dispatchTable.status == HubStatus.COMPUTED_PRIOR_LAYER : dispatchTable; dispatchTable.status = HubStatus.INSTALLED_CURRENT_LAYER; assert dispatchTable.slots.length == vtableLength : Assertions.errorMessage(vTable, dispatchTable.slots); @@ -415,7 +415,7 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec * Next determine if any vtable symbols defined in prior layers now have a resolved target. */ for (HostedDispatchTable typeInfo : typeToDispatchTable.values()) { - if (typeInfo.status == HubStatus.INSTALLED_PRIOR_LAYER) { + if (typeInfo.status == HubStatus.INSTALLED_PRIOR_LAYER || typeInfo.status == HubStatus.COMPUTED_PRIOR_LAYER) { for (HostedDispatchSlot slotInfo : typeInfo.slots) { if (slotInfo.status == SlotResolutionStatus.COMPUTED && slotInfo.resolvedMethod.isCompiled()) { /* @@ -449,6 +449,9 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec */ if (ImageLayerBuildingSupport.buildingApplicationLayer()) { priorUnresolvedSymbols.forEach(symbol -> { + if (symbol.isEmpty()) { + return; + } if (!resolvedPriorVTableMap.containsKey(symbol)) { CompilationResult result = codeCache.compilationResultFor(invalidMethod); @@ -519,6 +522,7 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { List dispatchTableInts = new ArrayList<>(); List dispatchSlotInts = new ArrayList<>(); List dispatchSlotStrings = new ArrayList<>(); + List dispatchTableBooleans = new ArrayList<>(); int nextSlotIdx = 0; for (HostedDispatchTable info : typeToDispatchTable.values()) { if (!layerWriter.isTypePersisted(info.type.getWrapped())) { @@ -555,6 +559,7 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { dispatchTableInts.add(slotOffsets.size()); dispatchTableInts.addAll(localTargets); dispatchTableInts.addAll(slotOffsets); + dispatchTableBooleans.add(info.type.getWrapped().isReachable()); } writer.writeIntList("dispatchTableInts", dispatchTableInts); @@ -563,6 +568,7 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { writer.writeBoolList("methodBooleans", methodBooleans); writer.writeIntList("methodInts", methodInts); writer.writeStringList("methodStrings", methodStrings); + writer.writeBoolList("dispatchTableBooleans", dispatchTableBooleans); return PersistFlags.CREATE; } @@ -575,6 +581,7 @@ public static Object createFromLoader(ImageSingletonLoader loader) { List methodBooleans = loader.readBoolList("methodBooleans"); List methodInts = loader.readIntList("methodInts"); List methodStrings = loader.readStringList("methodStrings"); + List dispatchTableBooleans = loader.readBoolList("dispatchTableBooleans"); Set unresolvedSymbols = new HashSet<>(); Map priorTypes = new HashMap<>(); @@ -618,8 +625,10 @@ public static Object createFromLoader(ImageSingletonLoader loader) { } intIterator = dispatchTableInts.iterator(); + boolIterator = dispatchTableBooleans.iterator(); while (intIterator.hasNext()) { int typeId = intIterator.next(); + boolean installed = boolIterator.next(); int locallyDeclaredMethodsSize = intIterator.next(); int allSlotsSize = intIterator.next(); PriorDispatchMethod[] locallyDeclaredSlots = new PriorDispatchMethod[locallyDeclaredMethodsSize]; @@ -631,7 +640,7 @@ public static Object createFromLoader(ImageSingletonLoader loader) { dispatchTableSlots[i] = priorDispatchSlots.get(intIterator.next()); } - var priorDispatchTable = new PriorDispatchTable(typeId, locallyDeclaredSlots, dispatchTableSlots); + var priorDispatchTable = new PriorDispatchTable(typeId, installed, locallyDeclaredSlots, dispatchTableSlots); Object prev = priorTypes.put(typeId, priorDispatchTable); assert prev == null : prev; @@ -649,6 +658,7 @@ public static Object createFromLoader(ImageSingletonLoader loader) { enum HubStatus { UNINITIALIZED, DISPATCH_INFO_CALCULATED, + COMPUTED_PRIOR_LAYER, INSTALLED_PRIOR_LAYER, INSTALLED_CURRENT_LAYER, } @@ -697,7 +707,7 @@ static class HostedDispatchSlot { * symbols. */ - record PriorDispatchTable(int typeID, + record PriorDispatchTable(int typeID, boolean installed, PriorDispatchMethod[] locallyDeclaredSlots, PriorDispatchSlot[] slots) { } From 9122983b310e38392c8b171f1a3b3ef438ec53bb Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Mon, 16 Dec 2024 15:19:24 +0100 Subject: [PATCH 2/2] Ensure only non null unresolved symbol are persisted in dispatch table --- .../svm/hosted/heap/SVMImageLayerWriter.java | 13 +++-- .../LayeredDispatchTableSupport.java | 50 +++++++++++-------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java index 7db4a4bf90d9..72aa935fa6dd 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.IntFunction; @@ -389,10 +390,7 @@ private static void writeImageSingletonKeyStore(ImageSingletonObject.Builder obj case String[] strl -> { TextList.Builder strlb = b.getValue().initStrl(strl.length); for (int j = 0; j < strl.length; j++) { - String s = strl[j]; - if (s != null) { - strlb.set(j, new Text.Reader(s)); - } + strlb.set(j, new Text.Reader(strl[j])); } } case boolean[] zl -> { @@ -414,8 +412,13 @@ EconomicMap getKeyValueStore() { return keyValueStore; } + private static boolean nonNullEntries(List list) { + return list.stream().filter(Objects::isNull).findAny().isEmpty(); + } + @Override public void writeBoolList(String keyName, List value) { + assert nonNullEntries(value); boolean[] b = new boolean[value.size()]; for (int i = 0; i < value.size(); i++) { b[i] = value.get(i); @@ -432,6 +435,7 @@ public void writeInt(String keyName, int value) { @Override public void writeIntList(String keyName, List value) { + assert nonNullEntries(value); var previous = keyValueStore.put(keyName, value.stream().mapToInt(i -> i).toArray()); assert previous == null : Assertions.errorMessage(keyName, previous); } @@ -450,6 +454,7 @@ public void writeString(String keyName, String value) { @Override public void writeStringList(String keyName, List value) { + assert nonNullEntries(value); var previous = keyValueStore.put(keyName, value.toArray(String[]::new)); assert previous == null : Assertions.errorMessage(keyName, previous); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java index 91e26d4ec619..b02bb0d2cfb7 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LayeredDispatchTableSupport.java @@ -229,12 +229,16 @@ private void injectPriorLayerInfo(HostedType type, HostedDispatchTable dispatchT if (priorInfo != null) { compareTypeInfo(dispatchTable, priorInfo); dispatchTable.status = priorInfo.installed ? HubStatus.INSTALLED_PRIOR_LAYER : HubStatus.COMPUTED_PRIOR_LAYER; - for (int i = 0; i < dispatchTable.slots.length; i++) { - HostedDispatchSlot slot = dispatchTable.slots[i]; - PriorDispatchSlot priorSlot = priorInfo.slots[i]; - if (priorSlot.status.isCompiled()) { - slot.status = SlotResolutionStatus.PRIOR_LAYER; - slot.symbol = priorSlot.slotSymbolName; + if (priorInfo.installed) { + // record symbol info for installed hubs + for (int i = 0; i < dispatchTable.slots.length; i++) { + HostedDispatchSlot slot = dispatchTable.slots[i]; + PriorDispatchSlot priorSlot = priorInfo.slots[i]; + if (priorSlot.status.isCompiled()) { + slot.status = SlotResolutionStatus.PRIOR_LAYER; + assert !priorSlot.slotSymbolName.equals(PriorDispatchSlot.INVALID_SYMBOL_NAME); + slot.symbol = priorSlot.slotSymbolName; + } } } } @@ -415,7 +419,7 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec * Next determine if any vtable symbols defined in prior layers now have a resolved target. */ for (HostedDispatchTable typeInfo : typeToDispatchTable.values()) { - if (typeInfo.status == HubStatus.INSTALLED_PRIOR_LAYER || typeInfo.status == HubStatus.COMPUTED_PRIOR_LAYER) { + if (typeInfo.status == HubStatus.INSTALLED_PRIOR_LAYER) { for (HostedDispatchSlot slotInfo : typeInfo.slots) { if (slotInfo.status == SlotResolutionStatus.COMPUTED && slotInfo.resolvedMethod.isCompiled()) { /* @@ -449,9 +453,6 @@ public void defineDispatchTableSlotSymbols(ObjectFile objectFile, ObjectFile.Sec */ if (ImageLayerBuildingSupport.buildingApplicationLayer()) { priorUnresolvedSymbols.forEach(symbol -> { - if (symbol.isEmpty()) { - return; - } if (!resolvedPriorVTableMap.containsKey(symbol)) { CompilationResult result = codeCache.compilationResultFor(invalidMethod); @@ -520,9 +521,9 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { * Write out dispatch tables for persisted types. */ List dispatchTableInts = new ArrayList<>(); + List dispatchTableBooleans = new ArrayList<>(); List dispatchSlotInts = new ArrayList<>(); List dispatchSlotStrings = new ArrayList<>(); - List dispatchTableBooleans = new ArrayList<>(); int nextSlotIdx = 0; for (HostedDispatchTable info : typeToDispatchTable.values()) { if (!layerWriter.isTypePersisted(info.type.getWrapped())) { @@ -538,9 +539,11 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { localTargets.add(methodToOffsetMapper.apply(hMethod)); } + boolean hubInstalled = info.status == HubStatus.INSTALLED_CURRENT_LAYER; if (info.slots != null) { for (var slotInfo : info.slots) { - dispatchSlotStrings.add(slotInfo.symbol); + var symbolName = hubInstalled ? slotInfo.symbol : PriorDispatchSlot.INVALID_SYMBOL_NAME; + dispatchSlotStrings.add(symbolName); dispatchSlotInts.add(slotInfo.slotNum); dispatchSlotInts.add(slotInfo.status.ordinal()); dispatchSlotInts.add(methodToOffsetMapper.apply(slotInfo.declaredMethod)); @@ -559,16 +562,16 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { dispatchTableInts.add(slotOffsets.size()); dispatchTableInts.addAll(localTargets); dispatchTableInts.addAll(slotOffsets); - dispatchTableBooleans.add(info.type.getWrapped().isReachable()); + dispatchTableBooleans.add(hubInstalled); } writer.writeIntList("dispatchTableInts", dispatchTableInts); + writer.writeBoolList("dispatchTableBooleans", dispatchTableBooleans); writer.writeIntList("dispatchSlotInts", dispatchSlotInts); writer.writeStringList("dispatchSlotStrings", dispatchSlotStrings); writer.writeBoolList("methodBooleans", methodBooleans); writer.writeIntList("methodInts", methodInts); writer.writeStringList("methodStrings", methodStrings); - writer.writeBoolList("dispatchTableBooleans", dispatchTableBooleans); return PersistFlags.CREATE; } @@ -576,12 +579,12 @@ public PersistFlags preparePersist(ImageSingletonWriter writer) { @SuppressWarnings("unused") public static Object createFromLoader(ImageSingletonLoader loader) { List dispatchTableInts = loader.readIntList("dispatchTableInts"); + List dispatchTableBooleans = loader.readBoolList("dispatchTableBooleans"); List dispatchSlotInts = loader.readIntList("dispatchSlotInts"); List dispatchSlotStrings = loader.readStringList("dispatchSlotStrings"); List methodBooleans = loader.readBoolList("methodBooleans"); List methodInts = loader.readIntList("methodInts"); List methodStrings = loader.readStringList("methodStrings"); - List dispatchTableBooleans = loader.readBoolList("dispatchTableBooleans"); Set unresolvedSymbols = new HashSet<>(); Map priorTypes = new HashMap<>(); @@ -616,10 +619,6 @@ public static Object createFromLoader(ImageSingletonLoader loader) { resolvedMethod = priorMethods.get(index); } - if (status == SlotResolutionStatus.UNRESOLVED || status == SlotResolutionStatus.NOT_COMPILED) { - unresolvedSymbols.add(slotSymbolName); - } - var slotInfo = new PriorDispatchSlot(declaredMethod, resolvedMethod, slotNum, status, slotSymbolName); priorDispatchSlots.add(slotInfo); } @@ -628,7 +627,7 @@ public static Object createFromLoader(ImageSingletonLoader loader) { boolIterator = dispatchTableBooleans.iterator(); while (intIterator.hasNext()) { int typeId = intIterator.next(); - boolean installed = boolIterator.next(); + boolean hubInstalled = boolIterator.next(); int locallyDeclaredMethodsSize = intIterator.next(); int allSlotsSize = intIterator.next(); PriorDispatchMethod[] locallyDeclaredSlots = new PriorDispatchMethod[locallyDeclaredMethodsSize]; @@ -638,9 +637,17 @@ public static Object createFromLoader(ImageSingletonLoader loader) { } for (int i = 0; i < allSlotsSize; i++) { dispatchTableSlots[i] = priorDispatchSlots.get(intIterator.next()); + if (hubInstalled) { + var status = dispatchTableSlots[i].status; + if (status == SlotResolutionStatus.UNRESOLVED || status == SlotResolutionStatus.NOT_COMPILED) { + assert !dispatchTableSlots[i].slotSymbolName.equals(PriorDispatchSlot.INVALID_SYMBOL_NAME); + unresolvedSymbols.add(dispatchTableSlots[i].slotSymbolName); + } + + } } - var priorDispatchTable = new PriorDispatchTable(typeId, installed, locallyDeclaredSlots, dispatchTableSlots); + var priorDispatchTable = new PriorDispatchTable(typeId, hubInstalled, locallyDeclaredSlots, dispatchTableSlots); Object prev = priorTypes.put(typeId, priorDispatchTable); assert prev == null : prev; @@ -715,6 +722,7 @@ record PriorDispatchSlot( PriorDispatchMethod declaredMethod, PriorDispatchMethod resolvedMethod, int slotNum, SlotResolutionStatus status, String slotSymbolName) { static final int UNKNOWN_METHOD = -1; + static final String INVALID_SYMBOL_NAME = "invalid"; } /**