[GR-53552] Remove Future from ImageLayerLoader

PullRequest: graal/17663
ziyilin · May 28, 2024 · 4aaf7e3 · 4aaf7e3
2 parents 99ceff5 + 61a8c4d
commit 4aaf7e3
Show file tree

Hide file tree

Showing 10 changed files with 236 additions and 310 deletions.
diff --git a/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java b/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerLoader.java
diff --git a/.../com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java b/.../com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java
@@ -58,6 +58,7 @@ public class ImageLayerSnapshotUtil {
     public static final String SUPER_CLASS_TAG = "super class";
     public static final String INTERFACES_TAG = "interfaces";
     public static final String CONSTANTS_TAG = "constants";
+    public static final String CONSTANTS_TO_RELINK_TAG = "constants to relink";
     public static final String TID_TAG = "tid";
     public static final String IDENTITY_HASH_CODE_TAG = "identityHashCode";
     public static final String ID_TAG = "id";

diff --git a/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java b/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java
@@ -29,6 +29,7 @@
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.CLASS_NAME_TAG;
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.COMPONENT_TYPE_TAG;
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.CONSTANTS_TAG;
+import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.CONSTANTS_TO_RELINK_TAG;
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.CONSTANT_TYPE_TAG;
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.DATA_TAG;
 import static com.oracle.graal.pointsto.heap.ImageLayerSnapshotUtil.ENCLOSING_TYPE_TAG;
@@ -102,6 +103,7 @@ public class ImageLayerWriter {
     private String[] imageInternedStrings;
 
     protected EconomicMap<String, Object> jsonMap = EconomicMap.create();
+    protected List<Integer> constantsToRelink;
     FileInfo fileInfo;
 
     private record FileInfo(Path layerSnapshotPath, String fileName, String suffix) {
@@ -113,6 +115,7 @@ public ImageLayerWriter() {
 
     public ImageLayerWriter(ImageLayerSnapshotUtil imageLayerSnapshotUtil) {
         this.imageLayerSnapshotUtil = imageLayerSnapshotUtil;
+        this.constantsToRelink = new ArrayList<>();
     }
 
     public void setImageInternedStrings(String[] imageInternedStrings) {
@@ -190,6 +193,7 @@ public void persistAnalysisInfo(Universe hostedUniverse, AnalysisUniverse analys
             }
         }
         jsonMap.put(CONSTANTS_TAG, constantsMap);
+        jsonMap.put(CONSTANTS_TO_RELINK_TAG, constantsToRelink);
     }
 
     /**
@@ -319,12 +323,12 @@ public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap
         boolean simulated = hostedObject == null;
         constantMap.put(SIMULATED_TAG, simulated);
         if (!simulated) {
-            persistConstantRelinkingInfo(constantMap, bb, clazz, hostedObject);
+            persistConstantRelinkingInfo(constantMap, bb, clazz, hostedObject, imageHeapConstant.constantData.id);
         }
     }
 
     @SuppressFBWarnings(value = "ES", justification = "Reference equality check needed to detect intern status")
-    public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject) {
+    public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject, int id) {
         if (clazz.equals(String.class)) {
             String value = bb.getSnippetReflectionProvider().asObject(String.class, hostedObject);
             int stringIndex = Arrays.binarySearch(imageInternedStrings, value);
@@ -334,11 +338,13 @@ public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap
              */
             if (stringIndex >= 0 && imageInternedStrings[stringIndex] == value) {
                 constantMap.put(VALUE_TAG, value);
+                constantsToRelink.add(id);
             }
         } else if (Enum.class.isAssignableFrom(clazz)) {
             Enum<?> value = bb.getSnippetReflectionProvider().asObject(Enum.class, hostedObject);
             constantMap.put(ENUM_CLASS_TAG, value.getDeclaringClass().getName());
             constantMap.put(ENUM_NAME_TAG, value.name());
+            constantsToRelink.add(id);
         }
     }
 

diff --git a/...ratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/...ratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java
@@ -274,10 +274,7 @@ public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKi
         }
 
         /* Set id after accessing super types, so that all these types get a lower id number. */
-        if (wrapped instanceof BaseLayerType baseLayerType) {
-            this.id = baseLayerType.getId();
-            this.isInBaseLayer = true;
-        } else if (universe.hostVM().useBaseLayer()) {
+        if (universe.hostVM().useBaseLayer()) {
             int tid = universe.getImageLayerLoader().lookupHostedTypeInBaseLayer(this);
             if (tid != -1) {
                 /*
@@ -288,7 +285,13 @@ public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKi
                 this.isInBaseLayer = true;
             } else {
                 this.id = universe.computeNextTypeId();
-                this.isInBaseLayer = false;
+                /*
+                 * If both the BaseLayerType and the complete type are created at the same time,
+                 * there can be a race for the base layer id. It is possible that the complete type
+                 * gets the base layer id even though the BaseLayerType is created. In this case,
+                 * the AnalysisType should still be marked as isInBaseLayer.
+                 */
+                this.isInBaseLayer = wrapped instanceof BaseLayerType;
             }
         } else {
             this.id = universe.computeNextTypeId();

diff --git a/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java b/...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java
@@ -222,13 +222,6 @@ public JavaType lookupAllowUnresolved(JavaType rawType) {
         if (result == null) {
             result = createType(type);
         }
-        if (result.isInBaseLayer()) {
-            /*
-             * The constants can only be relinked after the type is registered as the dynamic hub is
-             * not available otherwise.
-             */
-            getImageLayerLoader().loadAndRelinkTypeConstants(result);
-        }
         assert typesById[result.getId()].equals(result) : result;
         return result;
     }

diff --git a/...atevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/BaseLayerType.java b/...atevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/BaseLayerType.java
@@ -51,7 +51,7 @@ public class BaseLayerType implements ResolvedJavaType, OriginalClassProvider {
      */
     public static final String BASE_LAYER_SUFFIX = "_BaseLayer;";
     private final String name;
-    private final int id;
+    private final int baseLayerId;
     private final int modifiers;
     private final boolean isInterface;
     private final boolean isEnum;
@@ -64,10 +64,10 @@ public class BaseLayerType implements ResolvedJavaType, OriginalClassProvider {
     private final ResolvedJavaType[] interfaces;
     private final ResolvedJavaType objectType;
 
-    public BaseLayerType(String name, int id, int modifiers, boolean isInterface, boolean isEnum, boolean isInitialized, boolean isLinked, String sourceFileName, ResolvedJavaType enclosingType,
-                    ResolvedJavaType componentType, ResolvedJavaType superClass, ResolvedJavaType[] interfaces, ResolvedJavaType objectType) {
+    public BaseLayerType(String name, int baseLayerId, int modifiers, boolean isInterface, boolean isEnum, boolean isInitialized, boolean isLinked, String sourceFileName,
+                    ResolvedJavaType enclosingType, ResolvedJavaType componentType, ResolvedJavaType superClass, ResolvedJavaType[] interfaces, ResolvedJavaType objectType) {
         this.name = name.substring(0, name.length() - 1) + BASE_LAYER_SUFFIX;
-        this.id = id;
+        this.baseLayerId = baseLayerId;
         this.modifiers = modifiers;
         this.isInterface = isInterface;
         this.isEnum = isEnum;
@@ -328,7 +328,7 @@ public ResolvedJavaType unwrapTowardsOriginalType() {
         return objectType;
     }
 
-    public int getId() {
-        return id;
+    public int getBaseLayerId() {
+        return baseLayerId;
     }
 }
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
@@ -225,9 +225,9 @@
 import com.oracle.svm.hosted.code.CEntryPointData;
 import com.oracle.svm.hosted.code.CFunctionSubstitutionProcessor;
 import com.oracle.svm.hosted.code.CompileQueue;
-import com.oracle.svm.hosted.code.ObjectFileTransformer;
 import com.oracle.svm.hosted.code.HostedRuntimeConfigurationBuilder;
 import com.oracle.svm.hosted.code.NativeMethodSubstitutionProcessor;
+import com.oracle.svm.hosted.code.ObjectFileTransformer;
 import com.oracle.svm.hosted.code.RestrictHeapAccessCalleesImpl;
 import com.oracle.svm.hosted.code.SubstrateGraphMakerFactory;
 import com.oracle.svm.hosted.heap.ObservableImageHeapMapProviderImpl;
@@ -824,10 +824,6 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
                 bb.getHostVM().getClassInitializationSupport().setConfigurationSealed(true);
             }
 
-            if (ImageLayerBuildingSupport.buildingExtensionLayer()) {
-                HostedImageLayerBuildingSupport.singleton().getLoader().loadLayerConstants();
-            }
-
             try (ReporterClosable c = ProgressReporter.singleton().printAnalysis(bb.getUniverse(), nativeLibraries.getLibraries())) {
                 DuringAnalysisAccessImpl config = new DuringAnalysisAccessImpl(featureHandler, loader, bb, nativeLibraries, debug);
                 try {

diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java
@@ -64,6 +64,7 @@
 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.BaseLayerType;
 import com.oracle.graal.pointsto.meta.HostedProviders;
 import com.oracle.graal.pointsto.phases.InlineBeforeAnalysisGraphDecoder;
 import com.oracle.graal.pointsto.phases.InlineBeforeAnalysisPolicy;
@@ -408,7 +409,11 @@ private DynamicHub createHub(AnalysisType type) {
         ClassLoader hubClassLoader = javaClass.getClassLoader();
 
         /* Class names must be interned strings according to the Java specification. */
-        String className = type.toClassName().intern();
+        String name = type.toClassName();
+        if (name.endsWith(BaseLayerType.BASE_LAYER_SUFFIX.substring(0, BaseLayerType.BASE_LAYER_SUFFIX.length() - 1))) {
+            name = name.substring(0, name.length() - BaseLayerType.BASE_LAYER_SUFFIX.length() + 1);
+        }
+        String className = name.intern();
         /*
          * There is no need to have file names and simple binary names as interned strings. So we
          * perform our own de-duplication.

diff --git a/...tratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerLoader.java b/...tratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerLoader.java
@@ -74,6 +74,16 @@ public SVMImageLayerLoader(List<Path> loaderPaths) {
         dynamicHubArrayHubField = ReflectionUtil.lookupField(DynamicHub.class, "arrayHub");
     }
 
+    @Override
+    protected void prepareConstantRelinking(EconomicMap<String, Object> constantData, int id) {
+        Integer tid = get(constantData, CLASS_ID_TAG);
+        if (tid != null) {
+            typeToConstant.put(tid, id);
+        } else {
+            super.prepareConstantRelinking(constantData, id);
+        }
+    }
+
     @Override
     protected boolean delegateProcessing(String constantType, Object constantValue, Object[] values, int i) {
         if (constantType.equals(METHOD_POINTER_TAG)) {
@@ -112,31 +122,22 @@ protected boolean delegateProcessing(String constantType, Object constantValue,
     }
 
     @Override
-    protected void relinkConstant(ImageHeapInstance imageHeapInstance, EconomicMap<String, Object> baseLayerConstant, Class<?> clazz) {
+    protected JavaConstant getHostedObject(EconomicMap<String, Object> baseLayerConstant, Class<?> clazz) {
         if (clazz.equals(Class.class)) {
             Integer tid = get(baseLayerConstant, CLASS_ID_TAG);
             /* DynamicHub corresponding to $$TypeSwitch classes are not relinked */
             if (tid != null) {
-                if (universe.isTypeCreated(tid)) {
-                    relinkDynamicHub(imageHeapInstance, tid);
-                } else {
-                    /*
-                     * If the DynamicHub is not created yet, we create a task that will be executed
-                     * on the DynamicHub creation
-                     */
-                    AnalysisFuture<Void> task = new AnalysisFuture<>(() -> relinkDynamicHub(imageHeapInstance, tid));
-                    missingTypeTasks.computeIfAbsent(tid, unused -> ConcurrentHashMap.newKeySet()).add(task);
-                }
+                return getDynamicHub(tid);
             }
-        } else {
-            super.relinkConstant(imageHeapInstance, baseLayerConstant, clazz);
         }
+        return super.getHostedObject(baseLayerConstant, clazz);
     }
 
-    private void relinkDynamicHub(ImageHeapInstance imageHeapInstance, int tid) {
+    private JavaConstant getDynamicHub(int tid) {
+        getAnalysisType(tid);
         AnalysisType type = universe.getType(tid);
         DynamicHub hub = ((SVMHost) universe.hostVM()).dynamicHub(type);
-        relinkConstant(hub, imageHeapInstance);
+        return getHostedObject(hub);
     }
 
     @Override
@@ -185,6 +186,25 @@ public void rescanHub(AnalysisType type, Object hubObject) {
         }
     }
 
+    @Override
+    protected boolean hasValueForObject(Object object) {
+        if (object instanceof DynamicHub dynamicHub) {
+            AnalysisType type = ((SVMHost) universe.hostVM()).lookupType(dynamicHub);
+            return typeToConstant.containsKey(type.getId());
+        }
+        return super.hasValueForObject(object);
+    }
+
+    @Override
+    protected ImageHeapConstant getValueForObject(Object object) {
+        if (object instanceof DynamicHub dynamicHub) {
+            AnalysisType type = ((SVMHost) universe.hostVM()).lookupType(dynamicHub);
+            int id = typeToConstant.get(type.getId());
+            return getOrCreateConstant(id);
+        }
+        return super.getValueForObject(object);
+    }
+
     public Map<Object, Set<Class<?>>> loadImageSingletons(Object forbiddenObject) {
         loadJsonMap();
         return loadImageSingletons0(forbiddenObject);

diff --git a/...tratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java b/...tratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java
@@ -158,7 +158,7 @@ protected void persistConstant(AnalysisUniverse analysisUniverse, ImageHeapConst
     }
 
     @Override
-    public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject) {
+    public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject, int id) {
         ResolvedJavaType type = bb.getConstantReflectionProvider().asJavaType(hostedObject);
         if (type instanceof AnalysisType analysisType) {
             /*
@@ -170,9 +170,10 @@ public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap
              */
             if (!isTypeSwitch(analysisType)) {
                 constantMap.put(CLASS_ID_TAG, analysisType.getId());
+                constantsToRelink.add(id);
             }
         } else {
-            super.persistConstantRelinkingInfo(constantMap, bb, clazz, hostedObject);
+            super.persistConstantRelinkingInfo(constantMap, bb, clazz, hostedObject, id);
         }
     }