Skip to content

Commit 8d2de22

Browse files
ZeaveeZeavee
committed
[GR-57832] [GR-59694] Load strengthened graph early
PullRequest: graal/19150
2 parents 545c9ef + 6f91603 commit 8d2de22

File tree

16 files changed

+289
-58
lines changed

16 files changed

+289
-58
lines changed

‎substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/PointsToAnalyzer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ private PointsToAnalyzer(String mainEntryClass, OptionValues options) {
163163
aUniverse.setBigBang(bigbang);
164164
ImageHeap heap = new ImageHeap();
165165
HostedValuesProvider hostedValuesProvider = new HostedValuesProvider(aMetaAccess, aUniverse);
166-
ImageLayerSnapshotUtil imageLayerSnapshotUtil = new ImageLayerSnapshotUtil();
166+
ImageLayerSnapshotUtil imageLayerSnapshotUtil = new ImageLayerSnapshotUtil(false);
167167
ImageLayerLoader imageLayerLoader = new ImageLayerLoader();
168168
imageLayerLoader.setImageLayerSnapshotUtil(imageLayerSnapshotUtil);
169169
imageLayerLoader.setUniverse(aUniverse);
@@ -172,7 +172,7 @@ private PointsToAnalyzer(String mainEntryClass, OptionValues options) {
172172
snippetReflection, aConstantReflection, new AnalysisObjectScanningObserver(bigbang), analysisClassLoader, hostedValuesProvider);
173173
aUniverse.setHeapScanner(heapScanner);
174174
imageLayerLoader.executeHeapScannerTasks();
175-
ImageLayerWriter imageLayerWriter = new ImageLayerWriter(true);
175+
ImageLayerWriter imageLayerWriter = new ImageLayerWriter(true, true);
176176
imageLayerWriter.setImageLayerSnapshotUtil(imageLayerSnapshotUtil);
177177
imageLayerWriter.setImageHeap(heap);
178178
HeapSnapshotVerifier heapVerifier = new StandaloneHeapSnapshotVerifier(bigbang, heap, heapScanner);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,6 @@ public void cleanupAfterAnalysis() {
232232

233233
universe.getHeapScanner().cleanupAfterAnalysis();
234234
universe.getHeapVerifier().cleanupAfterAnalysis();
235-
if (universe.getImageLayerLoader() != null) {
236-
universe.getImageLayerLoader().cleanupAfterAnalysis();
237-
}
238235
}
239236

240237
@Override

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ public boolean enableTrackAcrossLayers() {
356356
return false;
357357
}
358358

359+
public boolean enableReachableInCurrentLayer() {
360+
return false;
361+
}
362+
359363
/**
360364
* Helpers to determine what analysis actions should be taken for a given Multi-Method version.
361365
*/

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,8 @@ protected void apply(boolean forceReparse, Object reason) {
698698
assert !processed : "can only call apply once per MethodTypeFlowBuilder";
699699
processed = true;
700700

701+
method.setReachableInCurrentLayer();
702+
701703
if (method.analyzedInPriorLayer()) {
702704
/*
703705
* We don't need to analyze this method. We already know its return type state from the

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

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
import jdk.graal.compiler.nodes.EncodedGraph;
9292
import jdk.graal.compiler.util.ObjectCopier;
9393
import jdk.vm.ci.meta.JavaConstant;
94+
import jdk.vm.ci.meta.JavaMethodProfile;
9495
import jdk.vm.ci.meta.MethodHandleAccessProvider.IntrinsicMethod;
9596
import jdk.vm.ci.meta.ResolvedJavaField;
9697
import jdk.vm.ci.meta.ResolvedJavaType;
@@ -179,7 +180,7 @@ public void loadLayerAnalysis() {
179180
loadLayerAnalysis0();
180181
}
181182

182-
public void cleanupAfterAnalysis() {
183+
public void cleanupAfterCompilation() {
183184
if (graphsChannel != null) {
184185
try {
185186
graphsChannel.close();
@@ -594,28 +595,33 @@ public void initializeBaseLayerMethod(AnalysisMethod analysisMethod) {
594595
* {@link ImageLayerWriter#persistAnalysisParsedGraph} for implementation.
595596
*/
596597
public boolean hasAnalysisParsedGraph(AnalysisMethod analysisMethod) {
597-
return getMethodData(analysisMethod).hasAnalysisGraphLocation();
598+
return hasGraph(analysisMethod, PersistedAnalysisMethod.Reader::hasAnalysisGraphLocation);
598599
}
599600

600601
public AnalysisParsedGraph getAnalysisParsedGraph(AnalysisMethod analysisMethod) {
601602
PersistedAnalysisMethod.Reader methodData = getMethodData(analysisMethod);
602603
boolean intrinsic = methodData.getAnalysisGraphIsIntrinsic();
603604
EncodedGraph analyzedGraph = getEncodedGraph(analysisMethod, methodData.getAnalysisGraphLocation());
604-
if (hasStrengthenedGraph(analysisMethod)) {
605-
throw AnalysisError.shouldNotReachHere("Strengthened graphs are not supported until late loading is implemented.");
606-
}
607605
return new AnalysisParsedGraph(analyzedGraph, intrinsic);
608606
}
609607

610608
public boolean hasStrengthenedGraph(AnalysisMethod analysisMethod) {
611-
return getMethodData(analysisMethod).hasStrengthenedGraphLocation();
609+
return hasGraph(analysisMethod, PersistedAnalysisMethod.Reader::hasStrengthenedGraphLocation);
612610
}
613611

614612
public EncodedGraph getStrengthenedGraph(AnalysisMethod analysisMethod) {
615613
PersistedAnalysisMethod.Reader methodData = getMethodData(analysisMethod);
616614
return getEncodedGraph(analysisMethod, methodData.getStrengthenedGraphLocation());
617615
}
618616

617+
private boolean hasGraph(AnalysisMethod analysisMethod, Function<PersistedAnalysisMethod.Reader, Boolean> hasGraphFunction) {
618+
var methodData = getMethodData(analysisMethod);
619+
if (methodData == null) {
620+
return false;
621+
}
622+
return hasGraphFunction.apply(methodData);
623+
}
624+
619625
protected EncodedGraph getEncodedGraph(AnalysisMethod analysisMethod, Text.Reader location) {
620626
byte[] encodedAnalyzedGraph = readEncodedGraph(location.toString());
621627
return (EncodedGraph) ObjectCopier.decode(imageLayerSnapshotUtil.getGraphDecoder(this, analysisMethod, universe.getSnippetReflection()), encodedAnalyzedGraph);
@@ -643,6 +649,65 @@ private byte[] readEncodedGraph(String location) {
643649
return bb.array();
644650
}
645651

652+
/**
653+
* This method is needed to ensure all the base layer analysis elements from the strengthened
654+
* graph are created early enough and seen by the analysis. This is done by decoding the graph
655+
* using a decoder that loads analysis elements instead of hosted elements.
656+
*/
657+
public void loadPriorStrengthenedGraphAnalysisElements(AnalysisMethod analysisMethod) {
658+
if (hasStrengthenedGraph(analysisMethod)) {
659+
PersistedAnalysisMethod.Reader methodData = getMethodData(analysisMethod);
660+
byte[] encodedAnalyzedGraph = readEncodedGraph(methodData.getStrengthenedGraphLocation().toString());
661+
EncodedGraph graph = (EncodedGraph) ObjectCopier.decode(imageLayerSnapshotUtil.getGraphAnalysisElementsDecoder(this, analysisMethod, universe.getSnippetReflection()),
662+
encodedAnalyzedGraph);
663+
for (Object o : graph.getObjects()) {
664+
if (o instanceof AnalysisMethod m) {
665+
m.setReachableInCurrentLayer();
666+
} else if (o instanceof JavaMethodProfile javaMethodProfile) {
667+
for (var m : javaMethodProfile.getMethods()) {
668+
if (m.getMethod() instanceof AnalysisMethod aMethod) {
669+
aMethod.setReachableInCurrentLayer();
670+
}
671+
}
672+
} else if (o instanceof ImageHeapConstant constant) {
673+
loadMaterializedChildren(constant);
674+
}
675+
}
676+
}
677+
}
678+
679+
private void loadMaterializedChildren(ImageHeapConstant constant) {
680+
if (constant instanceof ImageHeapInstance imageHeapInstance && !imageHeapInstance.nullFieldValues()) {
681+
loadMaterializedChildren(constant, imageHeapInstance.getFieldValues());
682+
} else if (constant instanceof ImageHeapObjectArray imageHeapObjectArray) {
683+
loadMaterializedChildren(constant, imageHeapObjectArray.getElementValues());
684+
}
685+
}
686+
687+
private void loadMaterializedChildren(ImageHeapConstant constant, Object[] values) {
688+
PersistedConstant.Reader baseLayerConstant = findConstant(ImageHeapConstant.getConstantID(constant));
689+
if (baseLayerConstant != null) {
690+
StructList.Reader<ConstantReference.Reader> data = baseLayerConstant.getObject().getData();
691+
for (int i = 0; i < data.size(); ++i) {
692+
ConstantReference.Reader childConstant = data.get(i);
693+
if (childConstant.isObjectConstant()) {
694+
if (childConstant.isNotMaterialized()) {
695+
continue;
696+
}
697+
loadMaterializedChild(values[i]);
698+
}
699+
}
700+
}
701+
}
702+
703+
private void loadMaterializedChild(Object child) {
704+
if (child instanceof AnalysisFuture<?> analysisFuture) {
705+
if (analysisFuture.ensureDone() instanceof ImageHeapConstant imageHeapConstant) {
706+
loadMaterializedChildren(imageHeapConstant);
707+
}
708+
}
709+
}
710+
646711
protected static int getId(String line) {
647712
return Integer.parseInt(line.split(" = ")[1]);
648713
}

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,15 @@ public class ImageLayerSnapshotUtil {
6868
/** This needs to be initialized after analysis, as some fields are not available before. */
6969
protected Map<Object, Field> externalValues;
7070

71-
public ImageLayerSnapshotUtil() {
72-
try {
73-
this.externalValueFields = ObjectCopier.getExternalValueFields();
74-
} catch (IOException e) {
75-
throw AnalysisError.shouldNotReachHere("Unexpected exception when creating external value fields list", e);
71+
public ImageLayerSnapshotUtil(boolean computeExternalValues) {
72+
if (computeExternalValues) {
73+
try {
74+
this.externalValueFields = ObjectCopier.getExternalValueFields();
75+
} catch (IOException e) {
76+
throw AnalysisError.shouldNotReachHere("Unexpected exception when creating external value fields list", e);
77+
}
78+
} else {
79+
this.externalValueFields = List.of();
7680
}
7781
}
7882

@@ -132,6 +136,11 @@ public GraphEncoder getGraphEncoder(ImageLayerWriter imageLayerWriter) {
132136
return new GraphEncoder(externalValues, imageLayerWriter);
133137
}
134138

139+
@SuppressWarnings("unused")
140+
public GraphDecoder getGraphAnalysisElementsDecoder(ImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) {
141+
return new GraphDecoder(EncodedGraph.class.getClassLoader(), imageLayerLoader, analysisMethod);
142+
}
143+
135144
@SuppressWarnings("unused")
136145
public GraphDecoder getGraphDecoder(ImageLayerLoader imageLayerLoader, AnalysisMethod analysisMethod, SnippetReflectionProvider snippetReflectionProvider) {
137146
return new GraphDecoder(EncodedGraph.class.getClassLoader(), imageLayerLoader, analysisMethod);

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,9 @@ void finish() {
188188
}
189189
}
190190

191-
public ImageLayerWriter() {
192-
this(true);
193-
}
194-
195-
public ImageLayerWriter(boolean useSharedLayerGraphs) {
191+
public ImageLayerWriter(boolean useSharedLayerGraphs, boolean useSharedLayerStrengthenedGraphs) {
196192
this.useSharedLayerGraphs = useSharedLayerGraphs;
197-
this.useSharedLayerStrengthenedGraphs = false;
193+
this.useSharedLayerStrengthenedGraphs = useSharedLayerStrengthenedGraphs;
198194
}
199195

200196
public void setImageLayerSnapshotUtil(ImageLayerSnapshotUtil imageLayerSnapshotUtil) {

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import com.oracle.graal.pointsto.api.PointstoOptions;
5454
import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
5555
import com.oracle.graal.pointsto.flow.AnalysisParsedGraph;
56+
import com.oracle.graal.pointsto.heap.ImageLayerLoader;
5657
import com.oracle.graal.pointsto.infrastructure.GraphProvider;
5758
import com.oracle.graal.pointsto.infrastructure.OriginalMethodProvider;
5859
import com.oracle.graal.pointsto.infrastructure.ResolvedSignature;
@@ -112,6 +113,9 @@ public abstract class AnalysisMethod extends AnalysisElement implements WrappedJ
112113
static final AtomicReferenceFieldUpdater<AnalysisMethod, Object> allImplementationsUpdater = AtomicReferenceFieldUpdater
113114
.newUpdater(AnalysisMethod.class, Object.class, "allImplementations");
114115

116+
private static final AtomicReferenceFieldUpdater<AnalysisMethod, Boolean> reachableInCurrentLayerUpdater = AtomicReferenceFieldUpdater
117+
.newUpdater(AnalysisMethod.class, Boolean.class, "reachableInCurrentLayer");
118+
115119
public record Signature(String name, AnalysisType[] parameterTypes) {
116120
}
117121

@@ -166,6 +170,8 @@ public record Signature(String name, AnalysisType[] parameterTypes) {
166170
@SuppressWarnings("unused") private volatile Object implementationInvokedNotifications;
167171
@SuppressWarnings("unused") private volatile Object isIntrinsicMethod;
168172
@SuppressWarnings("unused") private volatile Object isInlined;
173+
@SuppressWarnings("unused") private volatile Boolean reachableInCurrentLayer;
174+
private final boolean enableReachableInCurrentLayer;
169175

170176
private final AtomicReference<Object> parsedGraphCacheState = new AtomicReference<>(GRAPH_CACHE_UNPARSED);
171177
private static final Object GRAPH_CACHE_UNPARSED = "unparsed";
@@ -266,6 +272,8 @@ protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped,
266272
startTrackInvocations();
267273
}
268274
parsingContextMaxDepth = PointstoOptions.ParsingContextMaxDepth.getValue(declaringClass.universe.hostVM.options());
275+
276+
this.enableReachableInCurrentLayer = universe.hostVM.enableReachableInCurrentLayer();
269277
}
270278

271279
@SuppressWarnings("this-escape")
@@ -294,6 +302,8 @@ protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey)
294302
if (PointstoOptions.TrackAccessChain.getValue(declaringClass.universe.hostVM().options())) {
295303
startTrackInvocations();
296304
}
305+
306+
this.enableReachableInCurrentLayer = original.enableReachableInCurrentLayer;
297307
}
298308

299309
private static String createName(ResolvedJavaMethod wrapped, MultiMethodKey multiMethodKey) {
@@ -459,6 +469,21 @@ public boolean analyzedInPriorLayer() {
459469
return analyzedInPriorLayer;
460470
}
461471

472+
public boolean reachableInCurrentLayer() {
473+
return enableReachableInCurrentLayer && reachableInCurrentLayer != null && reachableInCurrentLayer;
474+
}
475+
476+
public void setReachableInCurrentLayer() {
477+
if (enableReachableInCurrentLayer && !reachableInCurrentLayer()) {
478+
AtomicUtils.atomicSetAndRun(this, true, reachableInCurrentLayerUpdater, () -> {
479+
ImageLayerLoader imageLayerLoader = getUniverse().getImageLayerLoader();
480+
if (imageLayerLoader != null) {
481+
imageLayerLoader.loadPriorStrengthenedGraphAnalysisElements(this);
482+
}
483+
});
484+
}
485+
}
486+
462487
/**
463488
* Registers this method as intrinsified to Graal nodes via a {@link InvocationPlugin graph
464489
* builder plugin}. Such a method is treated similar to an invoked method. For example, method

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,24 @@ public final void applyResults(AnalysisMethod method) {
234234
? ptaMethod.getTypeFlow().getMethodFlowsGraph().getNodeFlows().getKeys()
235235
: null;
236236
var debug = new DebugContext.Builder(bb.getOptions(), new GraalDebugHandlersFactory(bb.getSnippetReflectionProvider())).build();
237-
var graph = method.decodeAnalyzedGraph(debug, nodeReferences);
238-
if (graph == null) {
239-
return;
240-
}
241237

242238
if (method.analyzedInPriorLayer()) {
243239
/*
244-
* The method was already strengthened in a prior layer, so there is no need to
245-
* strengthen it in this layer.
240+
* The method was already strengthened in a prior layer. If the graph was persisted, it
241+
* will be loaded on demand during compilation, so there is no need to strengthen it in
242+
* this layer.
243+
*
244+
* GR-59646: The graphs from the base layer could be strengthened again in the
245+
* application layer using closed world assumptions.
246246
*/
247247
return;
248248
}
249249

250+
var graph = method.decodeAnalyzedGraph(debug, nodeReferences);
251+
if (graph == null) {
252+
return;
253+
}
254+
250255
graph.resetDebug(debug);
251256
if (beforeCounters != null) {
252257
beforeCounters.collect(graph);
@@ -503,6 +508,11 @@ public void simplify(Node n, SimplifierTool tool) {
503508
Stamp newStamp = strengthenStamp(oldStamp);
504509
if (newStamp != null) {
505510
LogicNode replacement = graph.addOrUniqueWithInputs(InstanceOfNode.createHelper((ObjectStamp) oldStamp.improveWith(newStamp), node.getValue(), node.profile(), node.getAnchor()));
511+
/*
512+
* GR-59681: Once isAssignable is implemented for BaseLayerType, this check can
513+
* be removed
514+
*/
515+
AnalysisError.guarantee(node != replacement, "The new stamp needs to be different from the old stamp");
506516
node.replaceAndDelete(replacement);
507517
tool.addToWorkList(replacement);
508518
}
@@ -560,7 +570,13 @@ public void simplify(Node n, SimplifierTool tool) {
560570
Stamp oldStamp = node.piStamp();
561571
Stamp newStamp = strengthenStamp(oldStamp);
562572
if (newStamp != null) {
563-
node.strengthenPiStamp(oldStamp.improveWith(newStamp));
573+
Stamp newPiStamp = oldStamp.improveWith(newStamp);
574+
/*
575+
* GR-59681: Once isAssignable is implemented for BaseLayerType, this check can
576+
* be removed
577+
*/
578+
AnalysisError.guarantee(!newPiStamp.equals(oldStamp), "The new stamp needs to be different from the old stamp");
579+
node.strengthenPiStamp(newPiStamp);
564580
tool.addToWorkList(node);
565581
}
566582
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,27 @@ public class SubstrateOptions {
109109
@Option(help = "Build shared library")//
110110
public static final HostedOptionKey<Boolean> SharedLibrary = new HostedOptionKey<>(false);
111111

112-
@Option(help = "Persist and reload graphs across layers. If false, graphs defined in the base layer can be reparsed by the current layer and inlined before analysis, " +
112+
@Option(help = "Persist and reload all graphs across layers. If false, graphs defined in the base layer can be reparsed by the current layer and inlined before analysis, " +
113113
"but will not be inlined after analysis has completed via our other inlining infrastructure")//
114114
public static final HostedOptionKey<Boolean> UseSharedLayerGraphs = new HostedOptionKey<>(true) {
115115
@Override
116116
protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean oldValue, Boolean newValue) {
117-
NeverInline.update(values, "SubstrateStringConcatHelper.simpleConcat");
117+
if (!newValue) {
118+
UseSharedLayerStrengthenedGraphs.update(values, false);
119+
}
120+
}
121+
};
122+
123+
@Option(help = "Persist and reload strengthened graphs across layers. If false, inlining after analysis will be disabled")//
124+
public static final HostedOptionKey<Boolean> UseSharedLayerStrengthenedGraphs = new HostedOptionKey<>(false) {
125+
@Override
126+
protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean oldValue, Boolean newValue) {
127+
if (newValue) {
128+
UserError.guarantee(UseSharedLayerStrengthenedGraphs.getValueOrDefault(values),
129+
"UseSharedLayerStrengthenedGraph is a subset of UseSharedLayerGraphs, so the former cannot be enabled alone.");
130+
} else {
131+
NeverInline.update(values, "SubstrateStringConcatHelper.simpleConcat");
132+
}
118133
}
119134
};
120135

0 commit comments

Comments
 (0)