Skip to content

Commit 4f67601

Browse files
author
Christian Wimmer
committed
[GR-39406] Simplify class initialization configuration for Lambda classes.
PullRequest: graal/15079
2 parents 0775f08 + c60b247 commit 4f67601

File tree

12 files changed

+151
-40
lines changed

12 files changed

+151
-40
lines changed

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/java/BytecodeParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2866,7 +2866,7 @@ protected ConstantNode appendConstant(JavaConstant constant) {
28662866
}
28672867

28682868
@Override
2869-
public <T extends ValueNode> T append(T v) {
2869+
public <T extends Node> T append(T v) {
28702870
assert !graph.trackNodeSourcePosition() || graph.currentNodeSourcePosition() != null || currentBlock == blockMap.getUnwindBlock() || currentBlock instanceof ExceptionDispatchBlock;
28712871
if (v.graph() != null) {
28722872
return v;
@@ -2878,7 +2878,7 @@ public <T extends ValueNode> T append(T v) {
28782878
return added;
28792879
}
28802880

2881-
private <T extends ValueNode> void updateLastInstruction(T v) {
2881+
private <T extends Node> void updateLastInstruction(T v) {
28822882
if (v instanceof FixedNode) {
28832883
FixedNode fixedNode = (FixedNode) v;
28842884
if (lastInstr != null) {

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.graalvm.compiler.core.common.type.StampFactory;
3636
import org.graalvm.compiler.core.common.type.StampPair;
3737
import org.graalvm.compiler.debug.GraalError;
38+
import org.graalvm.compiler.graph.Node;
3839
import org.graalvm.compiler.nodes.AbstractBeginNode;
3940
import org.graalvm.compiler.nodes.AbstractMergeNode;
4041
import org.graalvm.compiler.nodes.BeginNode;
@@ -91,7 +92,7 @@ public interface GraphBuilderContext extends GraphBuilderTool {
9192
*
9293
* @param kind the kind to use when type checking this operation
9394
* @param value the value to push to the stack. The value must already have been
94-
* {@linkplain #append(ValueNode) appended}.
95+
* {@linkplain #append(Node) appended}.
9596
*/
9697
void push(JavaKind kind, ValueNode value);
9798

@@ -122,14 +123,24 @@ default ValueNode[] popArguments(int argSize) {
122123
* type checking this operation.
123124
* @return a node equivalent to {@code value} in the graph
124125
*/
125-
default <T extends ValueNode> T add(T value) {
126+
default <T extends Node> T add(T value) {
126127
if (value.graph() != null) {
127128
assert !(value instanceof StateSplit) || ((StateSplit) value).stateAfter() != null;
128129
return value;
129130
}
130131
return setStateAfterIfNecessary(this, append(value));
131132
}
132133

134+
/**
135+
* Maybe performs canonicalization on the provided node. Either the result of the
136+
* canonicalization, or the original node if canonicalization is not possible, is added to the
137+
* graph and returned. Note that the return value can be null when canonicalization determines
138+
* that the node can be deleted.
139+
*/
140+
default Node canonicalizeAndAdd(Node value) {
141+
return add(value);
142+
}
143+
133144
default ValueNode addNonNullCast(ValueNode value) {
134145
AbstractPointerStamp valueStamp = (AbstractPointerStamp) value.stamp(NodeView.DEFAULT);
135146
if (valueStamp.nonNull()) {
@@ -537,7 +548,7 @@ default void replacePluginWithException(GeneratedInvocationPlugin plugin, Resolv
537548
throw GraalError.unimplementedParent(); // ExcludeFromJacocoGeneratedReport
538549
}
539550

540-
static <T extends ValueNode> T setStateAfterIfNecessary(GraphBuilderContext b, T value) {
551+
static <T extends Node> T setStateAfterIfNecessary(GraphBuilderContext b, T value) {
541552
if (value instanceof StateSplit) {
542553
StateSplit stateSplit = (StateSplit) value;
543554
FrameState oldState = stateSplit.stateAfter();

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderTool.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
package org.graalvm.compiler.nodes.graphbuilderconf;
2626

2727
import org.graalvm.compiler.debug.DebugContext;
28+
import org.graalvm.compiler.graph.Node;
2829
import org.graalvm.compiler.nodes.StructuredGraph;
29-
import org.graalvm.compiler.nodes.ValueNode;
3030
import org.graalvm.compiler.nodes.spi.CoreProviders;
3131
import org.graalvm.compiler.options.OptionValues;
3232

@@ -43,7 +43,7 @@ public interface GraphBuilderTool extends CoreProviders {
4343
* @param value the node to be added to the graph
4444
* @return either the node added or an equivalent node
4545
*/
46-
<T extends ValueNode> T append(T value);
46+
<T extends Node> T append(T value);
4747

4848
default Assumptions getAssumptions() {
4949
return getGraph().getAssumptions();

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/GraphKit.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.graalvm.compiler.debug.DebugContext;
4242
import org.graalvm.compiler.debug.GraalError;
4343
import org.graalvm.compiler.graph.Graph;
44+
import org.graalvm.compiler.graph.Node;
4445
import org.graalvm.compiler.graph.Node.ValueNumberable;
4546
import org.graalvm.compiler.graph.NodeSourcePosition;
4647
import org.graalvm.compiler.java.FrameStateBuilder;
@@ -153,9 +154,9 @@ public <T extends ValueNode> T add(T node) {
153154
return graph.add(changeToWord(node));
154155
}
155156

156-
public <T extends ValueNode> T changeToWord(T node) {
157-
if (wordTypes != null && wordTypes.isWord(node)) {
158-
node.setStamp(wordTypes.getWordStamp(StampTool.typeOrNull(node)));
157+
public <T extends Node> T changeToWord(T node) {
158+
if (node instanceof ValueNode valueNode && wordTypes != null && wordTypes.isWord(valueNode)) {
159+
valueNode.setStamp(wordTypes.getWordStamp(StampTool.typeOrNull(valueNode)));
159160
}
160161
return node;
161162
}
@@ -170,7 +171,7 @@ public final JavaKind asKind(JavaType type) {
170171
}
171172

172173
@Override
173-
public <T extends ValueNode> T append(T node) {
174+
public <T extends Node> T append(T node) {
174175
if (node.graph() != null) {
175176
return node;
176177
}

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/IntrinsicGraphBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.graalvm.compiler.debug.DebugCloseable;
3535
import org.graalvm.compiler.debug.DebugContext;
3636
import org.graalvm.compiler.debug.GraalError;
37+
import org.graalvm.compiler.graph.Node;
3738
import org.graalvm.compiler.graph.NodeSourcePosition;
3839
import org.graalvm.compiler.java.FrameStateBuilder;
3940
import org.graalvm.compiler.nodes.AbstractBeginNode;
@@ -166,7 +167,7 @@ protected IntrinsicGraphBuilder(OptionValues options,
166167
}
167168
}
168169

169-
private <T extends ValueNode> void updateLastInstruction(T v) {
170+
private <T extends Node> void updateLastInstruction(T v) {
170171
if (v instanceof FixedNode) {
171172
FixedNode fixedNode = (FixedNode) v;
172173
if (lastInstr != null) {
@@ -225,7 +226,7 @@ protected void mergeUnwinds() {
225226
}
226227

227228
@Override
228-
public <T extends ValueNode> T append(T v) {
229+
public <T extends Node> T append(T v) {
229230
if (v.graph() != null) {
230231
return v;
231232
}

‎compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/PEGraphDecoder.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
import org.graalvm.compiler.nodes.UnwindNode;
9696
import org.graalvm.compiler.nodes.ValueNode;
9797
import org.graalvm.compiler.nodes.WithExceptionNode;
98+
import org.graalvm.compiler.nodes.calc.FloatingNode;
9899
import org.graalvm.compiler.nodes.cfg.ControlFlowGraph;
99100
import org.graalvm.compiler.nodes.extended.AnchoringNode;
100101
import org.graalvm.compiler.nodes.extended.BytecodeExceptionNode;
@@ -422,7 +423,7 @@ public IntrinsicContext getIntrinsic() {
422423
}
423424

424425
@Override
425-
public <T extends ValueNode> T append(T value) {
426+
public <T extends Node> T append(T value) {
426427
throw unimplementedOverride(); // ExcludeFromJacocoGeneratedReport
427428
}
428429

@@ -551,7 +552,7 @@ public void setStateAfter(StateSplit stateSplit) {
551552

552553
@SuppressWarnings("try")
553554
@Override
554-
public <T extends ValueNode> T append(T v) {
555+
public <T extends Node> T append(T v) {
555556
if (v.graph() != null) {
556557
return v;
557558
}
@@ -564,6 +565,21 @@ public <T extends ValueNode> T append(T v) {
564565
}
565566
}
566567

568+
@Override
569+
public Node canonicalizeAndAdd(Node node) {
570+
Node canonicalized = node;
571+
if (canonicalized instanceof FixedNode fixedNode) {
572+
canonicalized = canonicalizeFixedNode(methodScope, null, fixedNode);
573+
} else if (canonicalized instanceof FloatingNode floatingNode) {
574+
canonicalized = handleFloatingNodeBeforeAdd(methodScope, null, floatingNode);
575+
}
576+
577+
if (canonicalized == null) {
578+
return null;
579+
}
580+
return super.canonicalizeAndAdd(canonicalized);
581+
}
582+
567583
private DebugCloseable withNodeSourcePosition() {
568584
if (getGraph().trackNodeSourcePosition()) {
569585
NodeSourcePosition callerBytecodePosition = methodScope.getCallerNodeSourcePosition();
@@ -574,7 +590,7 @@ private DebugCloseable withNodeSourcePosition() {
574590
return null;
575591
}
576592

577-
private <T extends ValueNode> void updateLastInstruction(T v) {
593+
private <T extends Node> void updateLastInstruction(T v) {
578594
if (v instanceof FixedNode) {
579595
FixedNode fixedNode = (FixedNode) v;
580596
if (lastInstr != null) {
@@ -722,7 +738,7 @@ public void setStateAfter(StateSplit sideEffect) {
722738

723739
@SuppressWarnings("try")
724740
@Override
725-
public <T extends ValueNode> T append(T v) {
741+
public <T extends Node> T append(T v) {
726742
if (v.graph() != null) {
727743
return v;
728744
}
@@ -745,7 +761,7 @@ private DebugCloseable withNodeSourcePosition() {
745761
return null;
746762
}
747763

748-
private <T extends ValueNode> void updateLastInstruction(T value) {
764+
private <T extends Node> void updateLastInstruction(T value) {
749765
if (value instanceof FixedWithNextNode) {
750766
FixedWithNextNode fixed = (FixedWithNextNode) value;
751767
graph.addBeforeFixed(insertBefore, fixed);

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/AllowAllHostedUsagesClassInitializationSupport.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
import java.lang.reflect.Proxy;
2828

29+
import org.graalvm.compiler.java.LambdaUtils;
30+
2931
import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
3032
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
3133
import com.oracle.svm.core.util.UserError;
@@ -159,9 +161,27 @@ InitKind computeInitKindAndMaybeInitializeClass(Class<?> clazz, boolean memoize)
159161
}
160162
superResult = superResult.max(processInterfaces(clazz, memoize));
161163

162-
if (superResult == InitKind.BUILD_TIME && Proxy.isProxyClass(clazz)) {
163-
forceInitializeHosted(clazz, "proxy classes with interfaces initialized at build time are also initialized at build time", false);
164-
return InitKind.BUILD_TIME;
164+
if (superResult == InitKind.BUILD_TIME && (Proxy.isProxyClass(clazz) || LambdaUtils.isLambdaType(metaAccess.lookupJavaType(clazz)))) {
165+
/*
166+
* To simplify class initialization configuration for proxy and lambda types,
167+
* registering all of their implemented interfaces as "initialize at build time" is
168+
* equivalent to registering the proxy/lambda type itself. This is safe because we know
169+
* that proxy/lambda types themselves have no problematic code in the class initializer
170+
* (they are generated classes).
171+
*
172+
* Note that we must look at all interfaces, including transitive dependencies.
173+
*/
174+
boolean allInterfacesSpecifiedAsBuildTime = true;
175+
for (Class<?> iface : allInterfaces(clazz)) {
176+
if (specifiedInitKindFor(iface) != InitKind.BUILD_TIME) {
177+
allInterfacesSpecifiedAsBuildTime = false;
178+
break;
179+
}
180+
}
181+
if (allInterfacesSpecifiedAsBuildTime) {
182+
forceInitializeHosted(clazz, "proxy/lambda classes with all interfaces explicitly marked as --initialize-at-build-time are also initialized at build time", false);
183+
return InitKind.BUILD_TIME;
184+
}
165185
}
166186

167187
InitKind result = superResult.max(clazzResult);

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ClassInitializationFeature.java

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static com.oracle.svm.hosted.classinitialization.InitKind.RUN_TIME;
3030

3131
import java.io.PrintWriter;
32+
import java.lang.reflect.Proxy;
3233
import java.util.ArrayList;
3334
import java.util.Arrays;
3435
import java.util.Comparator;
@@ -125,14 +126,42 @@ private Object checkImageHeapInstance(Object obj) {
125126
* means that the user cannot later manually register it as RERUN or RUN_TIME.
126127
*/
127128
if (obj != null && !classInitializationSupport.maybeInitializeAtBuildTime(obj.getClass())) {
128-
String msg = "No instances of " + obj.getClass().getTypeName() + " are allowed in the image heap as this class should be initialized at image runtime.";
129-
msg += classInitializationSupport.objectInstantiationTraceMessage(obj,
130-
" To fix the issue mark " + obj.getClass().getTypeName() + " for build-time initialization with " +
131-
SubstrateOptionsParser.commandArgument(ClassInitializationOptions.ClassInitialization, obj.getClass().getTypeName(), "initialize-at-build-time") +
132-
" or use the the information from the trace to find the culprit and " +
133-
SubstrateOptionsParser.commandArgument(ClassInitializationOptions.ClassInitialization, "<culprit>", "initialize-at-run-time") +
134-
" to prevent its instantiation.\n");
135-
throw new UnsupportedFeatureException(msg);
129+
StringBuilder msg = new StringBuilder()
130+
.append("No instances of ").append(obj.getClass().getTypeName()).append(" are allowed in the image heap as this class should be initialized at image runtime.");
131+
132+
String action = " To fix the issue mark " + obj.getClass().getTypeName() + " for build-time initialization with " +
133+
SubstrateOptionsParser.commandArgument(ClassInitializationOptions.ClassInitialization, obj.getClass().getTypeName(), "initialize-at-build-time") +
134+
" or use the the information from the trace to find the culprit and " +
135+
SubstrateOptionsParser.commandArgument(ClassInitializationOptions.ClassInitialization, "<culprit>", "initialize-at-run-time") +
136+
" to prevent its instantiation." + System.lineSeparator();
137+
msg.append(classInitializationSupport.objectInstantiationTraceMessage(obj, action));
138+
139+
if (!ClassInitializationOptions.UseDeprecatedOldClassInitialization.getValue()) {
140+
msg.append(System.lineSeparator())
141+
.append("If you see this error while migrating to a newer GraalVM release, please note that the class initialization strategy has changed in GraalVM for JDK 21.")
142+
.append(" All classes can now be used at image build time. However, only classes explicitly marked as --initialize-at-build-time are allowed to be in the image heap.")
143+
.append(" This rule is now strictly enforced, i.e., the problem might be solvable by registering the reported type as --initialize-at-build-time.");
144+
}
145+
146+
String proxyOrLambda = null;
147+
if (Proxy.isProxyClass(obj.getClass())) {
148+
proxyOrLambda = "Proxy";
149+
} else if (obj.getClass().getName().contains(LambdaUtils.LAMBDA_CLASS_NAME_SUBSTRING)) {
150+
proxyOrLambda = "Lambda";
151+
}
152+
if (proxyOrLambda != null) {
153+
msg.append(System.lineSeparator())
154+
.append("For ").append(proxyOrLambda).append(" classes, it is also sufficient to register all interfaces that the class implements as --initialize-at-build-time. ")
155+
.append(" The interfaces of this ").append(proxyOrLambda).append(" class are: [");
156+
String sep = "";
157+
for (var iface : ClassInitializationSupport.allInterfaces(obj.getClass())) {
158+
msg.append(sep).append(iface.getTypeName());
159+
sep = ", ";
160+
}
161+
msg.append("].");
162+
}
163+
164+
throw new UnsupportedFeatureException(msg.toString());
136165
}
137166
return obj;
138167
}
@@ -205,7 +234,7 @@ public void afterAnalysis(AfterAnalysisAccess a) {
205234
.filter(c -> c.getClassLoader() != null && c.getClassLoader() != ClassLoader.getPlatformClassLoader())
206235
.filter(c -> classInitializationSupport.specifiedInitKindFor(c) == null)
207236
.map(Class::getTypeName)
208-
.filter(name -> !LambdaUtils.isLambdaName(name))
237+
.filter(name -> !name.contains(LambdaUtils.LAMBDA_CLASS_NAME_SUBSTRING))
209238
.collect(Collectors.toList());
210239
if (!unspecifiedClasses.isEmpty()) {
211240
System.err.println("The following classes have unspecified initialization policy:" + System.lineSeparator() + String.join(System.lineSeparator(), unspecifiedClasses));

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ClassInitializationSupport.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.concurrent.ConcurrentMap;
3535
import java.util.stream.Collectors;
3636

37+
import org.graalvm.collections.EconomicSet;
3738
import org.graalvm.nativeimage.ImageSingletons;
3839
import org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport;
3940
import org.graalvm.nativeimage.impl.clinit.ClassInitializationTracking;
@@ -305,4 +306,18 @@ static String getTraceString(StackTraceElement[] trace) {
305306
abstract boolean checkDelayedInitialization();
306307

307308
abstract void doLateInitialization(AnalysisUniverse universe, AnalysisMetaAccess aMetaAccess);
309+
310+
public static EconomicSet<Class<?>> allInterfaces(Class<?> clazz) {
311+
EconomicSet<Class<?>> result = EconomicSet.create();
312+
addAllInterfaces(clazz, result);
313+
return result;
314+
}
315+
316+
private static void addAllInterfaces(Class<?> clazz, EconomicSet<Class<?>> result) {
317+
for (var interf : clazz.getInterfaces()) {
318+
if (result.add(interf)) {
319+
addAllInterfaces(interf, result);
320+
}
321+
}
322+
}
308323
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/DisallowedImageHeapObjectFeature.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@
4141

4242
import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
4343
import com.oracle.svm.core.SubstrateOptions;
44+
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
4445
import com.oracle.svm.core.feature.InternalFeature;
4546
import com.oracle.svm.core.image.DisallowedImageHeapObjects;
4647
import com.oracle.svm.core.jdk.management.ManagementFeature;
4748
import com.oracle.svm.core.jdk.management.ManagementSupport;
4849
import com.oracle.svm.core.option.SubstrateOptionsParser;
49-
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
5050
import com.oracle.svm.hosted.FeatureImpl;
5151
import com.oracle.svm.hosted.classinitialization.ClassInitializationOptions;
5252
import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
@@ -168,18 +168,11 @@ private void checkDisallowedMBeanObjects(Object original) {
168168
}
169169

170170
private RuntimeException error(String msg, Object obj, String initializerAction) {
171-
String suffix = "";
172-
if (!ClassInitializationOptions.UseDeprecatedOldClassInitialization.getValue()) {
173-
suffix = System.lineSeparator() +
174-
"If you see this error while migrating to a newer GraalVM release, please note that the class initialization strategy has changed in GraalVM for JDK 21." +
175-
" All classes can now be used at image build time. However, only classes explicitly marked as --initialize-at-built-time are allowed to be in the image heap." +
176-
" This rule is now strictly enforced, i.e., the problem might be solvable by registering the reported type as --initialize-at-built-time.";
177-
}
178171
throw new UnsupportedFeatureException(msg + " " + classInitialization.objectInstantiationTraceMessage(obj, initializerAction) + " " +
179172
"The object was probably created by a class initializer and is reachable from a static field. " +
180173
"You can request class initialization at image runtime by using the option " +
181174
SubstrateOptionsParser.commandArgument(ClassInitializationOptions.ClassInitialization, "<class-name>", "initialize-at-run-time") + ". " +
182-
"Or you can write your own initialization methods and call them explicitly from your main entry point." + suffix);
175+
"Or you can write your own initialization methods and call them explicitly from your main entry point.");
183176
}
184177

185178
private static boolean search(byte[] haystack, byte[] needle) {

0 commit comments

Comments
 (0)