[GR-59009] [GR-59571] Improve trackAcrossLayers

PullRequest: graal/19429

Author: Zeavee

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java

@@ -590,8 +590,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();

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<Field> externalValueFields;
     /** This needs to be initialized after analysis, as some fields are not available before. */
     protected Map<Object, Field> externalValues;
@@ -140,10 +142,10 @@ public static class GraphEncoder extends ObjectCopier.Encoder {
         public GraphEncoder(Map<Object, Field> 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,26 +216,19 @@ 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;
         }
 
         @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,20 +265,18 @@ 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;
         }
 
         @Override
         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();
     }
 

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java

@@ -121,38 +121,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<AnalysisType> typesToPersist = ConcurrentHashMap.newKeySet();
-    private final Set<AnalysisMethod> methodsToPersist = ConcurrentHashMap.newKeySet();
-    private final Set<AnalysisField> fieldsToPersist = ConcurrentHashMap.newKeySet();
     private final Set<ImageHeapConstant> 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);
     }
@@ -164,7 +135,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);
 
@@ -254,6 +226,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();
 
@@ -293,6 +270,10 @@ public void persistAnalysisInfo() {
         snapshotBuilder.setNextFieldId(aUniverse.getNextFieldId());
         snapshotBuilder.setNextConstantId(ImageHeapConstant.getCurrentId());
 
+        List<AnalysisType> typesToPersist = aUniverse.getTypes().stream().filter(AnalysisType::isTrackedAcrossLayers).toList();
+        List<AnalysisMethod> methodsToPersist = aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers).toList();
+        List<AnalysisField> 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);
@@ -347,7 +328,7 @@ protected void persistHook() {
     }
 
     public boolean isTypePersisted(AnalysisType type) {
-        return typesToPersist.contains(type);
+        return type.isTrackedAcrossLayers();
     }
 
     private void persistType(AnalysisType type, Supplier<PersistedAnalysisType.Builder> builderSupplier) {
@@ -375,12 +356,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. */
         }
@@ -401,6 +376,8 @@ protected void persistType(AnalysisType type, String typeDescriptor, PersistedAn
         builder.setIsReachable(type.isReachable());
 
         imageLayerWriterHelper.persistType(type, builder);
+
+        afterTypeAdded(type);
     }
 
     protected static void initInts(IntFunction<PrimitiveList.Int.Builder> builderSupplier, IntStream ids) {
@@ -419,31 +396,17 @@ protected static void initStringList(IntFunction<TextList.Builder> 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);
         }
     }
 
@@ -537,31 +500,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<AnalysisMethod> 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) {
@@ -649,7 +606,9 @@ protected void persistField(AnalysisField field, PersistedAnalysisField.Builder
     protected void persistConstant(ImageHeapConstant imageHeapConstant, ConstantParent parent, PersistedConstant.Builder builder, Set<Integer> 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);

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) {
     }
 }