Skip to content

Commit 07d4cfd

Browse files
ZeaveeZeavee
committed
[GR-60551] [GR-60503] Mark all persisted constants as reachable
PullRequest: graal/19744
2 parents 7cc02b1 + 6719253 commit 07d4cfd

File tree

10 files changed

+119
-70
lines changed

10 files changed

+119
-70
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/EncodedGraph.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,22 @@ public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClas
106106
sourceGraph.trackNodeSourcePosition());
107107
}
108108

109+
public EncodedGraph(EncodedGraph original) {
110+
this(original.encoding,
111+
original.startOffset,
112+
original.objects,
113+
original.types,
114+
original.assumptions,
115+
original.inlinedMethods,
116+
original.hasUnsafeAccess,
117+
original.trackNodeSourcePosition);
118+
/*
119+
* The nodeStartOffsets is only used as a cache for decoding, so it is not copied to ensure
120+
* not having inconsistent caching information in the new EncodedGraph.
121+
*/
122+
this.nodeStartOffsets = null;
123+
}
124+
109125
public EncodedGraph(byte[] encoding, int startOffset, Object[] objects, NodeClass<?>[] types, Assumptions assumptions, List<ResolvedJavaMethod> inlinedMethods,
110126
boolean hasUnsafeAccess, boolean trackNodeSourcePosition) {
111127
this.encoding = encoding;

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ public static class OtherReason extends ScanReason {
546546
public static final ScanReason UNKNOWN = new OtherReason("manually created constant");
547547
public static final ScanReason RESCAN = new OtherReason("manually triggered rescan");
548548
public static final ScanReason HUB = new OtherReason("scanning a class constant");
549+
public static final ScanReason PERSISTED = new OtherReason("persisted");
549550

550551
final String reason;
551552

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/ImageLayerWriter.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
*/
2525
package com.oracle.graal.pointsto.api;
2626

27+
import com.oracle.graal.pointsto.flow.AnalysisParsedGraph;
2728
import com.oracle.graal.pointsto.meta.AnalysisMethod;
2829
import com.oracle.graal.pointsto.util.AnalysisError;
2930

@@ -33,4 +34,9 @@ public class ImageLayerWriter {
3334
public void onTrackedAcrossLayer(AnalysisMethod method, Object reason) {
3435
throw AnalysisError.shouldNotReachHere("This method should not be called");
3536
}
37+
38+
@SuppressWarnings("unused")
39+
public void persistAnalysisParsedGraph(AnalysisMethod method, AnalysisParsedGraph analysisParsedGraph) {
40+
throw AnalysisError.shouldNotReachHere("This method should not be called");
41+
}
3642
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ private void notifyAnalysis(AnalysisField field, ImageHeapInstance receiver, Jav
497497
}
498498

499499
private void notifyAnalysis(AnalysisField field, ImageHeapInstance receiver, JavaConstant fieldValue, ScanReason reason, Consumer<ScanReason> onAnalysisModified) {
500-
if (scanningObserver != null) {
500+
if (!universe.sealed()) {
501501
/* Notify the points-to analysis of the scan. */
502502
boolean analysisModified = doNotifyAnalysis(field, receiver, fieldValue, reason);
503503
if (analysisModified && onAnalysisModified != null) {
@@ -563,7 +563,7 @@ private boolean notifyAnalysis(JavaConstant array, AnalysisType arrayType, JavaC
563563
return analysisModified;
564564
}
565565

566-
protected JavaConstant markReachable(JavaConstant constant, ScanReason reason) {
566+
public JavaConstant markReachable(JavaConstant constant, ScanReason reason) {
567567
return markReachable(constant, reason, null);
568568
}
569569

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.Map;
4040
import java.util.Set;
4141
import java.util.concurrent.ConcurrentHashMap;
42+
import java.util.concurrent.atomic.AtomicBoolean;
4243
import java.util.concurrent.atomic.AtomicReference;
4344
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
4445
import java.util.concurrent.locks.ReentrantLock;
@@ -50,6 +51,7 @@
5051

5152
import com.oracle.graal.pointsto.BigBang;
5253
import com.oracle.graal.pointsto.api.ImageLayerLoader;
54+
import com.oracle.graal.pointsto.api.ImageLayerWriter;
5355
import com.oracle.graal.pointsto.api.PointstoOptions;
5456
import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
5557
import com.oracle.graal.pointsto.flow.AnalysisParsedGraph;
@@ -174,6 +176,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) {
174176
private final boolean enableReachableInCurrentLayer;
175177

176178
private final AtomicReference<GraphCacheEntry> parsedGraphCacheState = new AtomicReference<>(GraphCacheEntry.UNPARSED);
179+
private final AtomicBoolean trackedGraphPersisted = new AtomicBoolean(false);
177180

178181
private EncodedGraph analyzedGraph;
179182

@@ -547,6 +550,13 @@ protected void notifyImplementationInvokedCallbacks() {
547550
ConcurrentLightHashSet.removeElementIf(this, implementationInvokedNotificationsUpdater, ElementNotification::isNotified);
548551
}
549552

553+
private void persistTrackedGraph(AnalysisParsedGraph graph) {
554+
if (isTrackedAcrossLayers() && trackedGraphPersisted.compareAndSet(false, true)) {
555+
ImageLayerWriter imageLayerWriter = getUniverse().getImageLayerWriter();
556+
imageLayerWriter.persistAnalysisParsedGraph(this, graph);
557+
}
558+
}
559+
550560
/** Get the set of all callers for this method, as inferred by the static analysis. */
551561
public Set<AnalysisMethod> getCallers() {
552562
return getInvokeLocations().stream().map(location -> (AnalysisMethod) location.getMethod()).collect(Collectors.toSet());
@@ -684,6 +694,10 @@ protected void onTrackedAcrossLayers(Object reason) {
684694
parameter.registerAsTrackedAcrossLayers(reason);
685695
}
686696
signature.getReturnType().registerAsTrackedAcrossLayers(reason);
697+
698+
if (getParsedGraphCacheStateObject() instanceof AnalysisParsedGraph analysisParsedGraph) {
699+
persistTrackedGraph(analysisParsedGraph);
700+
}
687701
}
688702

689703
public void registerOverrideReachabilityNotification(MethodOverrideReachableNotification notification) {
@@ -1233,7 +1247,13 @@ private AnalysisParsedGraph setGraph(BigBang bb, Stage stage, GraphCacheEntry ex
12331247
boolean result = parsedGraphCacheState.compareAndSet(lockState, newEntry);
12341248
AnalysisError.guarantee(result, "State transition failed");
12351249

1236-
return (AnalysisParsedGraph) newEntry.get(stage);
1250+
AnalysisParsedGraph analysisParsedGraph = (AnalysisParsedGraph) newEntry.get(stage);
1251+
1252+
if (stage == Stage.finalStage()) {
1253+
persistTrackedGraph(analysisParsedGraph);
1254+
}
1255+
1256+
return analysisParsedGraph;
12371257

12381258
} catch (Throwable ex) {
12391259
parsedGraphCacheState.set(GraphCacheEntry.createParsingError(stage, expectedValue, ex));

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/ImageLayerBuildingSupport.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@ protected ImageLayerBuildingSupport(boolean buildingImageLayer, boolean building
7878
*/
7979
public static void openModules() {
8080
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.lang");
81+
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.lang.reflect");
8182
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.util");
8283
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "java.util.concurrent");
84+
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, "java.base", "sun.reflect.annotation");
8385
}
8486

8587
private static ImageLayerBuildingSupport singleton() {

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -572,11 +572,6 @@ protected void doRun(Map<Method, CEntryPointData> entryPoints, JavaMainSupport j
572572
return;
573573
}
574574

575-
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
576-
HostedImageLayerBuildingSupport.singleton().getWriter().initializeExternalValues();
577-
HostedImageLayerBuildingSupport.singleton().getWriter().persistAnalysisParsedGraphs();
578-
}
579-
580575
NativeImageHeap heap;
581576
HostedMetaAccess hMetaAccess;
582577
RuntimeConfiguration runtimeConfiguration;
@@ -811,6 +806,9 @@ protected boolean runPointsToAnalysis(String imageName, OptionValues options, De
811806
if (ImageLayerBuildingSupport.buildingImageLayer()) {
812807
ImageSingletons.lookup(LoadImageSingletonFeature.class).processRegisteredSingletons(aUniverse);
813808
}
809+
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
810+
HostedImageLayerBuildingSupport.singleton().getWriter().initializeExternalValues();
811+
}
814812
}
815813

816814
try (ReporterClosable c = ProgressReporter.singleton().printAnalysis(bb.getUniverse(), nativeLibraries.getLibraries())) {

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,9 @@ private PersistedAnalysisMethod.Reader getMethodData(AnalysisMethod analysisMeth
823823
}
824824

825825
/**
826-
* See {@link SVMImageLayerWriter#persistAnalysisParsedGraphs()} for implementation.
826+
* See
827+
* {@link SVMImageLayerWriter#persistAnalysisParsedGraph(AnalysisMethod, AnalysisParsedGraph)}
828+
* for implementation.
827829
*/
828830
@Override
829831
public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) {
@@ -1248,11 +1250,11 @@ private Object[] getReferencedValues(ImageHeapConstant parentConstant, StructLis
12481250
if (delegateProcessing(constantData, values, position)) {
12491251
continue;
12501252
}
1253+
int finalPosition = position;
12511254
values[position] = switch (constantData.which()) {
12521255
case OBJECT_CONSTANT -> {
12531256
int constantId = constantData.getObjectConstant().getConstantId();
12541257
boolean relink = positionsToRelink.contains(position);
1255-
int finalPosition = position;
12561258
yield new AnalysisFuture<>(() -> {
12571259
ensureHubInitialized(parentConstant);
12581260

@@ -1279,7 +1281,14 @@ private Object[] getReferencedValues(ImageHeapConstant parentConstant, StructLis
12791281
* in the base image.
12801282
*/
12811283
new AnalysisFuture<>(() -> {
1282-
throw AnalysisError.shouldNotReachHere("This constant was not materialized in the base image.");
1284+
String errorMessage = "Reading the value of a base layer constant which was not materialized in the base image, ";
1285+
if (parentConstant instanceof ImageHeapInstance instance) {
1286+
AnalysisField field = getFieldFromIndex(instance, finalPosition);
1287+
errorMessage += "reachable by reading field " + field + " of parent object constant: " + parentConstant;
1288+
} else {
1289+
errorMessage += "reachable by indexing at position " + finalPosition + " into parent array constant: " + parentConstant;
1290+
}
1291+
throw AnalysisError.shouldNotReachHere(errorMessage);
12831292
});
12841293
case PRIMITIVE_VALUE -> {
12851294
PrimitiveValue.Reader pv = constantData.getPrimitiveValue();

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
import org.graalvm.nativeimage.ImageSingletons;
4444

45+
import com.oracle.graal.pointsto.ObjectScanner;
4546
import com.oracle.graal.pointsto.heap.ImageHeapConstant;
4647
import com.oracle.graal.pointsto.heap.ImageHeapInstance;
4748
import com.oracle.graal.pointsto.heap.ImageHeapObjectArray;
@@ -51,6 +52,7 @@
5152
import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
5253
import com.oracle.graal.pointsto.meta.AnalysisMethod;
5354
import com.oracle.graal.pointsto.meta.AnalysisType;
55+
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
5456
import com.oracle.graal.pointsto.meta.PointsToAnalysisField;
5557
import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod;
5658
import com.oracle.graal.pointsto.meta.PointsToAnalysisType;
@@ -84,6 +86,7 @@
8486
import jdk.graal.compiler.util.ObjectCopierInputStream;
8587
import jdk.graal.compiler.util.ObjectCopierOutputStream;
8688
import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod;
89+
import jdk.vm.ci.meta.ConstantReflectionProvider;
8790

8891
public class SVMImageLayerSnapshotUtil {
8992
public static final String FILE_NAME_PREFIX = "layer-snapshot-";
@@ -115,6 +118,8 @@ public class SVMImageLayerSnapshotUtil {
115118
protected static final Set<Field> dynamicHubRelinkedFields = Set.of(companion, name, componentType);
116119
protected static final Set<Field> dynamicHubCompanionRelinkedFields = Set.of(classInitializationInfo, superHub, arrayHub);
117120

121+
private static final Class<?> sourceRoots = ReflectionUtil.lookupClass("com.oracle.svm.hosted.image.sources.SourceCache$SourceRoots");
122+
118123
/**
119124
* This map stores the field indexes that should be relinked using the hosted value of a
120125
* constant from the key type.
@@ -151,6 +156,10 @@ private void addSVMExternalValueFields() {
151156
continue;
152157
}
153158

159+
if (!shouldScanClass(clazz)) {
160+
continue;
161+
}
162+
154163
/* The ObjectCopier needs to access the static fields by reflection */
155164
Module module = clazz.getModule();
156165
ModuleSupport.accessPackagesToClass(ModuleSupport.Access.OPEN, ObjectCopier.class, false, module.getName(), packageName);
@@ -182,6 +191,11 @@ protected boolean shouldScanPackage(String packageName) {
182191
return true;
183192
}
184193

194+
private static boolean shouldScanClass(Class<?> clazz) {
195+
/* This class should not be scanned because it needs to be initialized after the analysis */
196+
return !clazz.equals(sourceRoots);
197+
}
198+
185199
/**
186200
* Get all the field indexes that should be relinked using the hosted value of a constant from
187201
* the given type.
@@ -207,8 +221,8 @@ private static Set<Integer> getRelinkedFields(AnalysisType type, Set<Field> type
207221
return typeRelinkedFieldsSet.stream().map(metaAccess::lookupJavaField).map(AnalysisField::getPosition).collect(Collectors.toSet());
208222
}
209223

210-
public SVMGraphEncoder getGraphEncoder(SVMImageLayerWriter imageLayerWriter) {
211-
return new SVMGraphEncoder(externalValues, imageLayerWriter);
224+
public SVMGraphEncoder getGraphEncoder() {
225+
return new SVMGraphEncoder(externalValues);
212226
}
213227

214228
public AbstractSVMGraphDecoder getGraphHostedToAnalysisElementsDecoder(SVMImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) {
@@ -309,9 +323,9 @@ private static String getQualifiedName(AnalysisMethod method) {
309323

310324
public static class SVMGraphEncoder extends ObjectCopier.Encoder {
311325
@SuppressWarnings("this-escape")
312-
public SVMGraphEncoder(Map<Object, Field> externalValues, SVMImageLayerWriter imageLayerWriter) {
326+
public SVMGraphEncoder(Map<Object, Field> externalValues) {
313327
super(externalValues);
314-
addBuiltin(new ImageHeapConstantBuiltIn(imageLayerWriter, null));
328+
addBuiltin(new ImageHeapConstantBuiltIn(null));
315329
addBuiltin(new AnalysisTypeBuiltIn(null));
316330
addBuiltin(new AnalysisMethodBuiltIn(null, null));
317331
addBuiltin(new AnalysisFieldBuiltIn(null));
@@ -333,7 +347,7 @@ public abstract static class AbstractSVMGraphDecoder extends ObjectCopier.Decode
333347
public AbstractSVMGraphDecoder(ClassLoader classLoader, SVMImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) {
334348
super(classLoader);
335349
this.imageLayerBuildingSupport = imageLayerLoader.getImageLayerBuildingSupport();
336-
addBuiltin(new ImageHeapConstantBuiltIn(null, imageLayerLoader));
350+
addBuiltin(new ImageHeapConstantBuiltIn(imageLayerLoader));
337351
addBuiltin(new AnalysisTypeBuiltIn(imageLayerLoader));
338352
addBuiltin(new AnalysisMethodBuiltIn(imageLayerLoader, analysisMethod));
339353
addBuiltin(new AnalysisFieldBuiltIn(imageLayerLoader));
@@ -371,19 +385,28 @@ public SVMGraphDecoder(ClassLoader classLoader, SVMImageLayerLoader svmImageLaye
371385
}
372386

373387
public static class ImageHeapConstantBuiltIn extends ObjectCopier.Builtin {
374-
private final SVMImageLayerWriter imageLayerWriter;
375388
private final SVMImageLayerLoader imageLayerLoader;
376389

377-
protected ImageHeapConstantBuiltIn(SVMImageLayerWriter imageLayerWriter, SVMImageLayerLoader imageLayerLoader) {
390+
protected ImageHeapConstantBuiltIn(SVMImageLayerLoader imageLayerLoader) {
378391
super(ImageHeapConstant.class, ImageHeapInstance.class, ImageHeapObjectArray.class, ImageHeapPrimitiveArray.class);
379-
this.imageLayerWriter = imageLayerWriter;
380392
this.imageLayerLoader = imageLayerLoader;
381393
}
382394

383395
@Override
384396
public void encode(ObjectCopier.Encoder encoder, ObjectCopierOutputStream stream, Object obj) throws IOException {
385397
ImageHeapConstant imageHeapConstant = (ImageHeapConstant) obj;
386-
imageLayerWriter.ensureConstantPersisted(imageHeapConstant);
398+
399+
AnalysisUniverse universe = imageHeapConstant.getType().getUniverse();
400+
universe.getHeapScanner().markReachable(imageHeapConstant, ObjectScanner.OtherReason.PERSISTED);
401+
402+
imageHeapConstant.getType().registerAsTrackedAcrossLayers(imageHeapConstant);
403+
/* If this is a Class constant persist the corresponding type. */
404+
ConstantReflectionProvider constantReflection = universe.getBigbang().getConstantReflectionProvider();
405+
AnalysisType typeFromClassConstant = (AnalysisType) constantReflection.asJavaType(imageHeapConstant);
406+
if (typeFromClassConstant != null) {
407+
typeFromClassConstant.registerAsTrackedAcrossLayers(imageHeapConstant);
408+
}
409+
387410
stream.writePackedUnsignedInt(ImageHeapConstant.getConstantID(imageHeapConstant));
388411
}
389412

0 commit comments

Comments
 (0)