Skip to content

Commit 958786b

Browse files
lahodajlahodaj
committed
8332522: SwitchBootstraps::mappedEnumLookup constructs unused array
Reviewed-by: liach, redestad
1 parent f92c60e commit 958786b

File tree

2 files changed

+178
-39
lines changed

2 files changed

+178
-39
lines changed

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

+73-39
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656

5757
import static java.lang.invoke.MethodHandles.Lookup.ClassOption.NESTMATE;
5858
import static java.lang.invoke.MethodHandles.Lookup.ClassOption.STRONG;
59+
import java.util.Arrays;
5960
import java.util.HashMap;
6061
import java.util.Map;
6162
import static java.util.Objects.requireNonNull;
@@ -95,19 +96,13 @@ private SwitchBootstraps() {}
9596
private static final ClassDesc CD_Objects = ReferenceClassDescImpl.ofValidated("Ljava/util/Objects;");
9697

9798
private static class StaticHolders {
98-
private static final MethodHandle NULL_CHECK;
99-
private static final MethodHandle IS_ZERO;
100-
private static final MethodHandle MAPPED_ENUM_LOOKUP;
99+
private static final MethodHandle MAPPED_ENUM_SWITCH;
101100

102101
static {
103102
try {
104-
NULL_CHECK = LOOKUP.findStatic(Objects.class, "isNull",
105-
MethodType.methodType(boolean.class, Object.class));
106-
IS_ZERO = LOOKUP.findStatic(SwitchBootstraps.class, "isZero",
107-
MethodType.methodType(boolean.class, int.class));
108-
MAPPED_ENUM_LOOKUP = LOOKUP.findStatic(SwitchBootstraps.class, "mappedEnumLookup",
109-
MethodType.methodType(int.class, Enum.class, MethodHandles.Lookup.class,
110-
Class.class, EnumDesc[].class, EnumMap.class));
103+
MAPPED_ENUM_SWITCH = LOOKUP.findStatic(SwitchBootstraps.class, "mappedEnumSwitch",
104+
MethodType.methodType(int.class, Enum.class, int.class, MethodHandles.Lookup.class,
105+
Class.class, EnumDesc[].class, MappedEnumCache.class));
111106
}
112107
catch (ReflectiveOperationException e) {
113108
throw new ExceptionInInitializerError(e);
@@ -211,10 +206,6 @@ private static void verifyLabel(Object label, Class<?> selectorType) {
211206
}
212207
}
213208

214-
private static boolean isZero(int value) {
215-
return value == 0;
216-
}
217-
218209
/**
219210
* Bootstrap method for linking an {@code invokedynamic} call site that
220211
* implements a {@code switch} on a target of an enum type. The static
@@ -286,23 +277,27 @@ public static CallSite enumSwitch(MethodHandles.Lookup lookup,
286277
labels = labels.clone();
287278

288279
Class<?> enumClass = invocationType.parameterType(0);
289-
labels = Stream.of(labels).map(l -> convertEnumConstants(lookup, enumClass, l)).toArray();
280+
boolean constantsOnly = true;
281+
int len = labels.length;
282+
283+
for (int i = 0; i < len; i++) {
284+
Object convertedLabel =
285+
convertEnumConstants(lookup, enumClass, labels[i]);
286+
labels[i] = convertedLabel;
287+
if (constantsOnly)
288+
constantsOnly = convertedLabel instanceof EnumDesc;
289+
}
290290

291291
MethodHandle target;
292-
boolean constantsOnly = Stream.of(labels).allMatch(l -> enumClass.isAssignableFrom(EnumDesc.class));
293292

294293
if (labels.length > 0 && constantsOnly) {
295294
//If all labels are enum constants, construct an optimized handle for repeat index 0:
296295
//if (selector == null) return -1
297296
//else if (idx == 0) return mappingArray[selector.ordinal()]; //mapping array created lazily
298297
//else return "typeSwitch(labels)"
299-
MethodHandle body =
300-
MethodHandles.guardWithTest(MethodHandles.dropArguments(StaticHolders.NULL_CHECK, 0, int.class),
301-
MethodHandles.dropArguments(MethodHandles.constant(int.class, -1), 0, int.class, Object.class),
302-
MethodHandles.guardWithTest(MethodHandles.dropArguments(StaticHolders.IS_ZERO, 1, Object.class),
303-
generateTypeSwitch(lookup, invocationType.parameterType(0), labels),
304-
MethodHandles.insertArguments(StaticHolders.MAPPED_ENUM_LOOKUP, 1, lookup, enumClass, labels, new EnumMap())));
305-
target = MethodHandles.permuteArguments(body, MethodType.methodType(int.class, Object.class, int.class), 1, 0);
298+
EnumDesc<?>[] enumDescLabels =
299+
Arrays.copyOf(labels, labels.length, EnumDesc[].class);
300+
target = MethodHandles.insertArguments(StaticHolders.MAPPED_ENUM_SWITCH, 2, lookup, enumClass, enumDescLabels, new MappedEnumCache());
306301
} else {
307302
target = generateTypeSwitch(lookup, invocationType.parameterType(0), labels);
308303
}
@@ -331,26 +326,63 @@ private static <E extends Enum<E>> Object convertEnumConstants(MethodHandles.Loo
331326
}
332327
}
333328

334-
private static <T extends Enum<T>> int mappedEnumLookup(T value, MethodHandles.Lookup lookup, Class<T> enumClass, EnumDesc<?>[] labels, EnumMap enumMap) {
335-
if (enumMap.map == null) {
336-
T[] constants = SharedSecrets.getJavaLangAccess().getEnumConstantsShared(enumClass);
337-
int[] map = new int[constants.length];
338-
int ordinal = 0;
339-
340-
for (T constant : constants) {
341-
map[ordinal] = labels.length;
329+
private static <T extends Enum<T>> int mappedEnumSwitch(T value, int restartIndex, MethodHandles.Lookup lookup, Class<T> enumClass, EnumDesc<?>[] labels, MappedEnumCache enumCache) throws Throwable {
330+
if (value == null) {
331+
return -1;
332+
}
342333

343-
for (int i = 0; i < labels.length; i++) {
344-
if (Objects.equals(labels[i].constantName(), constant.name())) {
345-
map[ordinal] = i;
346-
break;
334+
if (restartIndex != 0) {
335+
MethodHandle generatedSwitch = enumCache.generatedSwitch;
336+
if (generatedSwitch == null) {
337+
synchronized (enumCache) {
338+
generatedSwitch = enumCache.generatedSwitch;
339+
340+
if (generatedSwitch == null) {
341+
generatedSwitch =
342+
generateTypeSwitch(lookup, enumClass, labels)
343+
.asType(MethodType.methodType(int.class,
344+
Enum.class,
345+
int.class));
346+
enumCache.generatedSwitch = generatedSwitch;
347347
}
348348
}
349+
}
350+
351+
return (int) generatedSwitch.invokeExact(value, restartIndex);
352+
}
353+
354+
int[] constantsMap = enumCache.constantsMap;
355+
356+
if (constantsMap == null) {
357+
synchronized (enumCache) {
358+
constantsMap = enumCache.constantsMap;
349359

350-
ordinal++;
360+
if (constantsMap == null) {
361+
T[] constants = SharedSecrets.getJavaLangAccess()
362+
.getEnumConstantsShared(enumClass);
363+
constantsMap = new int[constants.length];
364+
int ordinal = 0;
365+
366+
for (T constant : constants) {
367+
constantsMap[ordinal] = labels.length;
368+
369+
for (int i = 0; i < labels.length; i++) {
370+
if (Objects.equals(labels[i].constantName(),
371+
constant.name())) {
372+
constantsMap[ordinal] = i;
373+
break;
374+
}
375+
}
376+
377+
ordinal++;
378+
}
379+
380+
enumCache.constantsMap = constantsMap;
381+
}
351382
}
352383
}
353-
return enumMap.map[value.ordinal()];
384+
385+
return constantsMap[value.ordinal()];
354386
}
355387

356388
private static final class ResolvedEnumLabels implements BiPredicate<Integer, Object> {
@@ -395,9 +427,11 @@ public boolean test(Integer labelIndex, Object value) {
395427
}
396428
}
397429

398-
private static final class EnumMap {
430+
private static final class MappedEnumCache {
431+
@Stable
432+
public int[] constantsMap;
399433
@Stable
400-
public int[] map;
434+
public MethodHandle generatedSwitch;
401435
}
402436

403437
/*

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

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
package org.openjdk.bench.java.lang.runtime;
24+
25+
import org.openjdk.jmh.annotations.Benchmark;
26+
import org.openjdk.jmh.annotations.BenchmarkMode;
27+
import org.openjdk.jmh.annotations.Fork;
28+
import org.openjdk.jmh.annotations.Measurement;
29+
import org.openjdk.jmh.annotations.Mode;
30+
import org.openjdk.jmh.annotations.OutputTimeUnit;
31+
import org.openjdk.jmh.annotations.Scope;
32+
import org.openjdk.jmh.annotations.Setup;
33+
import org.openjdk.jmh.annotations.State;
34+
import org.openjdk.jmh.annotations.Warmup;
35+
36+
import java.util.concurrent.TimeUnit;
37+
38+
@BenchmarkMode(Mode.AverageTime)
39+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
40+
@State(Scope.Thread)
41+
@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
42+
@Measurement(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
43+
@Fork(3)
44+
public class SwitchEnum {
45+
46+
public E[] inputs;
47+
@Setup
48+
public void setup() {
49+
inputs = E.values();
50+
}
51+
52+
@Benchmark
53+
public int enumSwitchWithBootstrap() {
54+
int sum = 0;
55+
for (E e : inputs) {
56+
sum += switch (e) {
57+
case null -> -1;
58+
case E0 -> 10;
59+
case E1 -> 11;
60+
case E2 -> 12;
61+
case E3 -> 13;
62+
case E4 -> 14;
63+
case E5 -> 15;
64+
case E6 -> 16;
65+
case E7 -> 17;
66+
case E8 -> 18;
67+
case E9 -> 19;
68+
default -> 17;
69+
};
70+
}
71+
return sum;
72+
}
73+
74+
@Benchmark
75+
public int enumSwitchTraditional() {
76+
int sum = 0;
77+
for (E e : inputs) {
78+
sum += switch (e) {
79+
case E0 -> 10;
80+
case E1 -> 11;
81+
case E2 -> 12;
82+
case E3 -> 13;
83+
case E4 -> 14;
84+
case E5 -> 15;
85+
case E6 -> 16;
86+
case E7 -> 17;
87+
case E8 -> 18;
88+
case E9 -> 19;
89+
default -> 17;
90+
};
91+
}
92+
return sum;
93+
}
94+
95+
public static void main(String[] args) {
96+
SwitchEnum s = new SwitchEnum();
97+
s.setup();
98+
System.out.println(s.enumSwitchWithBootstrap());
99+
System.out.println(s.enumSwitchTraditional());
100+
}
101+
102+
enum E {
103+
E0, E1, E2, E3, E4, E5, E6, E7, E8, E9;
104+
}
105+
}

0 commit comments

Comments
 (0)