On Fri, 17 Mar 2023 12:15:58 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
> This is the first draft of a patch for JEP 440 and JEP 441. Changes included:
>
> - the pattern matching for switch and record patterns features are made
> final, together with updates to tests.
> - parenthesized patterns are removed.
> - qualified enum constants are supported for case labels.
>
> This change herein also includes removal record patterns in for each loop,
> which may be split into a separate PR in the future.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 1:
> 1: /*
@lahodaj The way that the boostrap method `enumSwitch` and its corresponding
implementation method `doEnumSwitch` are implemented right now has an
unfortunate side effect on class initialization: since the bootstrap method
already looks up the actual enum values, it triggers class initialization of
the enum. This is not a problem when the switched value is non-null, because
then obviously the enum is already initialized. But when the switched value is
`null`, then the boostrap method triggers class initialization.
Example:
enum E {
A, B;
static {
new Throwable("Hello, World!").printStackTrace();
}
}
public class SwitchTest {
public static int testMethod(E selExpr) {
return switch (selExpr) {
case A -> 3;
case B -> 6;
case null -> -1;
};
}
public static void main(String argv[]) {
System.out.println(testMethod(null));
}
}
It produces the following output (on JDK 20, but I don't see any changes in
this PR that would alter the behavior):
java.lang.Throwable: Hello, World!
at E.<clinit>(SwitchTest.java:5)
at java.base/jdk.internal.misc.Unsafe.ensureClassInitialized0(Native
Method)
at
java.base/jdk.internal.misc.Unsafe.ensureClassInitialized(Unsafe.java:1160)
at
java.base/jdk.internal.reflect.MethodHandleAccessorFactory.ensureClassInitialized(MethodHandleAccessorFactory.java:300)
at
java.base/jdk.internal.reflect.MethodHandleAccessorFactory.newMethodAccessor(MethodHandleAccessorFactory.java:71)
at
java.base/jdk.internal.reflect.ReflectionFactory.newMethodAccessor(ReflectionFactory.java:159)
at
java.base/java.lang.reflect.Method.acquireMethodAccessor(Method.java:724)
at java.base/java.lang.reflect.Method.invoke(Method.java:575)
at java.base/java.lang.Class.getEnumConstantsShared(Class.java:3938)
at java.base/java.lang.Class.enumConstantDirectory(Class.java:3961)
at java.base/java.lang.Enum.valueOf(Enum.java:268)
at
java.base/java.lang.invoke.ConstantBootstraps.enumConstant(ConstantBootstraps.java:144)
at
java.base/java.lang.runtime.SwitchBootstraps.convertEnumConstants(SwitchBootstraps.java:262)
at
java.base/java.lang.runtime.SwitchBootstraps.lambda$enumSwitch$0(SwitchBootstraps.java:238)
at
java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at
java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1006)
at
java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at
java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at
java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575)
at
java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260)
at
java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616)
at
java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622)
at
java.base/java.lang.runtime.SwitchBootstraps.enumSwitch(SwitchBootstraps.java:238)
at
java.base/java.lang.invoke.BootstrapMethodInvoker.invoke(BootstrapMethodInvoker.java:147)
at java.base/java.lang.invoke.CallSite.makeSite(CallSite.java:316)
at
java.base/java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:279)
at
java.base/java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:269)
at SwitchTest.testMethod(SwitchTest.java:13)
at SwitchTest.main(SwitchTest.java:21)
-1
I argue this is bad because in general because every unnecessary class
initialization (and its side effects) should be avoided.
But it is really a problem for any kind of eager-bootstrapping system, like
Project Leyden or GraalVM Native Image: if it is not possible to execute the
bootstrap methods early due to possible side effects, optimizations are quite
limited and the bootstrap method needs to be executed at run time.
A simple solution would be to keep the elements in the data array as `String`
instead of enum elements, and in `doEnumSwitch` do a `== target.name` (the
method has the comment `// Dumbest possible strategy` anyways).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13074#discussion_r1164570743
