8332522: SwitchBootstraps::mappedEnumLookup constructs unused array

Reviewed-by: liach, redestad

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java

@@ -56,6 +56,7 @@ import jdk.internal.vm.annotation.Stable;
 
 import static java.lang.invoke.MethodHandles.Lookup.ClassOption.NESTMATE;
 import static java.lang.invoke.MethodHandles.Lookup.ClassOption.STRONG;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import static java.util.Objects.requireNonNull;
@@ -95,19 +96,13 @@ public class SwitchBootstraps {
     private static final ClassDesc CD_Objects = ReferenceClassDescImpl.ofValidated("Ljava/util/Objects;");
 
     private static class StaticHolders {
-        private static final MethodHandle NULL_CHECK;
+        private static final MethodHandle MAPPED_ENUM_SWITCH;
-        private static final MethodHandle IS_ZERO;
-        private static final MethodHandle MAPPED_ENUM_LOOKUP;
 
         static {
             try {
-                NULL_CHECK = LOOKUP.findStatic(Objects.class, "isNull",
+                MAPPED_ENUM_SWITCH = LOOKUP.findStatic(SwitchBootstraps.class, "mappedEnumSwitch",
-                                               MethodType.methodType(boolean.class, Object.class));
+                                                       MethodType.methodType(int.class, Enum.class, int.class, MethodHandles.Lookup.class,
-                IS_ZERO = LOOKUP.findStatic(SwitchBootstraps.class, "isZero",
+                                                                             Class.class, EnumDesc[].class, MappedEnumCache.class));
-                                               MethodType.methodType(boolean.class, int.class));
-                MAPPED_ENUM_LOOKUP = LOOKUP.findStatic(SwitchBootstraps.class, "mappedEnumLookup",
-                                                       MethodType.methodType(int.class, Enum.class, MethodHandles.Lookup.class,
-                                                                             Class.class, EnumDesc[].class, EnumMap.class));
             }
             catch (ReflectiveOperationException e) {
                 throw new ExceptionInInitializerError(e);
@@ -211,10 +206,6 @@ public class SwitchBootstraps {
         }
     }
 
-    private static boolean isZero(int value) {
-        return value == 0;
-    }
-
     /**
      * Bootstrap method for linking an {@code invokedynamic} call site that
      * implements a {@code switch} on a target of an enum type. The static
@@ -286,23 +277,27 @@ public class SwitchBootstraps {
         labels = labels.clone();
 
         Class<?> enumClass = invocationType.parameterType(0);
-        labels = Stream.of(labels).map(l -> convertEnumConstants(lookup, enumClass, l)).toArray();
+        boolean constantsOnly = true;
+        int len = labels.length;
+
+        for (int i = 0; i < len; i++) {
+            Object convertedLabel =
+                    convertEnumConstants(lookup, enumClass, labels[i]);
+            labels[i] = convertedLabel;
+            if (constantsOnly)
+                constantsOnly = convertedLabel instanceof EnumDesc;
+        }
 
         MethodHandle target;
-        boolean constantsOnly = Stream.of(labels).allMatch(l -> enumClass.isAssignableFrom(EnumDesc.class));
 
         if (labels.length > 0 && constantsOnly) {
             //If all labels are enum constants, construct an optimized handle for repeat index 0:
             //if (selector == null) return -1
             //else if (idx == 0) return mappingArray[selector.ordinal()]; //mapping array created lazily
             //else return "typeSwitch(labels)"
-            MethodHandle body =
+            EnumDesc<?>[] enumDescLabels =
-                    MethodHandles.guardWithTest(MethodHandles.dropArguments(StaticHolders.NULL_CHECK, 0, int.class),
+                    Arrays.copyOf(labels, labels.length, EnumDesc[].class);
-                                                MethodHandles.dropArguments(MethodHandles.constant(int.class, -1), 0, int.class, Object.class),
+            target = MethodHandles.insertArguments(StaticHolders.MAPPED_ENUM_SWITCH, 2, lookup, enumClass, enumDescLabels, new MappedEnumCache());
-                                                MethodHandles.guardWithTest(MethodHandles.dropArguments(StaticHolders.IS_ZERO, 1, Object.class),
-                                                                            generateTypeSwitch(lookup, invocationType.parameterType(0), labels),
-                                                                            MethodHandles.insertArguments(StaticHolders.MAPPED_ENUM_LOOKUP, 1, lookup, enumClass, labels, new EnumMap())));
-            target = MethodHandles.permuteArguments(body, MethodType.methodType(int.class, Object.class, int.class), 1, 0);
         } else {
             target = generateTypeSwitch(lookup, invocationType.parameterType(0), labels);
         }
@@ -331,26 +326,63 @@ public class SwitchBootstraps {
         }
     }
 
-    private static <T extends Enum<T>> int mappedEnumLookup(T value, MethodHandles.Lookup lookup, Class<T> enumClass, EnumDesc<?>[] labels, EnumMap enumMap) {
+    private static <T extends Enum<T>> int mappedEnumSwitch(T value, int restartIndex, MethodHandles.Lookup lookup, Class<T> enumClass, EnumDesc<?>[] labels, MappedEnumCache enumCache) throws Throwable {
-        if (enumMap.map == null) {
+        if (value == null) {
-            T[] constants = SharedSecrets.getJavaLangAccess().getEnumConstantsShared(enumClass);
+            return -1;
-            int[] map = new int[constants.length];
+        }
-            int ordinal = 0;
 
-            for (T constant : constants) {
+        if (restartIndex != 0) {
-                map[ordinal] = labels.length;
+            MethodHandle generatedSwitch = enumCache.generatedSwitch;
+            if (generatedSwitch == null) {
+                synchronized (enumCache) {
+                    generatedSwitch = enumCache.generatedSwitch;
 
-                for (int i = 0; i < labels.length; i++) {
+                    if (generatedSwitch == null) {
-                    if (Objects.equals(labels[i].constantName(), constant.name())) {
+                        generatedSwitch =
-                        map[ordinal] = i;
+                                generateTypeSwitch(lookup, enumClass, labels)
-                        break;
+                                        .asType(MethodType.methodType(int.class,
+                                                                      Enum.class,
+                                                                      int.class));
+                        enumCache.generatedSwitch = generatedSwitch;
                     }
                 }
+            }
 
-                ordinal++;
+            return (int) generatedSwitch.invokeExact(value, restartIndex);
+        }
+
+        int[] constantsMap = enumCache.constantsMap;
+
+        if (constantsMap == null) {
+            synchronized (enumCache) {
+                constantsMap = enumCache.constantsMap;
+
+                if (constantsMap == null) {
+                    T[] constants = SharedSecrets.getJavaLangAccess()
+                                                 .getEnumConstantsShared(enumClass);
+                    constantsMap = new int[constants.length];
+                    int ordinal = 0;
+
+                    for (T constant : constants) {
+                        constantsMap[ordinal] = labels.length;
+
+                        for (int i = 0; i < labels.length; i++) {
+                            if (Objects.equals(labels[i].constantName(),
+                                               constant.name())) {
+                                constantsMap[ordinal] = i;
+                                break;
+                            }
+                        }
+
+                        ordinal++;
+                    }
+
+                    enumCache.constantsMap = constantsMap;
+                }
             }
         }
-        return enumMap.map[value.ordinal()];
+
+        return constantsMap[value.ordinal()];
     }
 
     private static final class ResolvedEnumLabels implements BiPredicate<Integer, Object> {
@@ -395,9 +427,11 @@ public class SwitchBootstraps {
         }
     }
 
-    private static final class EnumMap {
+    private static final class MappedEnumCache {
         @Stable
-        public int[] map;
+        public int[] constantsMap;
+        @Stable
+        public MethodHandle generatedSwitch;
     }
 
     /*

test/micro/org/openjdk/bench/java/lang/runtime/SwitchEnum.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.bench.java.lang.runtime;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.util.concurrent.TimeUnit;
+
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+public class SwitchEnum {
+
+    public E[] inputs;
+    @Setup
+    public void setup() {
+        inputs = E.values();
+    }
+
+    @Benchmark
+    public int enumSwitchWithBootstrap() {
+        int sum = 0;
+        for (E e : inputs) {
+            sum += switch (e) {
+                case null -> -1;
+                case E0 -> 10;
+                case E1 -> 11;
+                case E2 -> 12;
+                case E3 -> 13;
+                case E4 -> 14;
+                case E5 -> 15;
+                case E6 -> 16;
+                case E7 -> 17;
+                case E8 -> 18;
+                case E9 -> 19;
+                default -> 17;
+            };
+        }
+        return sum;
+    }
+
+    @Benchmark
+    public int enumSwitchTraditional() {
+        int sum = 0;
+        for (E e : inputs) {
+            sum += switch (e) {
+                case E0 -> 10;
+                case E1 -> 11;
+                case E2 -> 12;
+                case E3 -> 13;
+                case E4 -> 14;
+                case E5 -> 15;
+                case E6 -> 16;
+                case E7 -> 17;
+                case E8 -> 18;
+                case E9 -> 19;
+                default -> 17;
+            };
+        }
+        return sum;
+    }
+
+    public static void main(String[] args) {
+        SwitchEnum s = new SwitchEnum();
+        s.setup();
+        System.out.println(s.enumSwitchWithBootstrap());
+        System.out.println(s.enumSwitchTraditional());
+    }
+
+    enum E {
+        E0, E1, E2, E3, E4, E5, E6, E7, E8, E9;
+    }
+}