RFR: 8319220: Pattern matching switch with a lot of cases is unduly slow [v2]

[Jan Lahoda]

On Fri, 3 Nov 2023 09:48:15 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Jan Lahoda has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Some more get->orElseThrow
>>  - Reflecting review feedback.
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 338:
> 
>> 336: 
>> 337:         @Override
>> 338:         public Object apply(Integer labelIndex, Object value) {
> 
> Since we already know the EnumDesc (i.e. `result`), we can just convert this to a `BiPredicate<Integer, Object>` for simplicity.

Done.

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 346:
> 
>> 344:                     Class<?> clazz = label.constantType().resolveConstantDesc(lookup);
>> 345: 
>> 346:                     if (value.getClass() != clazz) {
> 
> Not related to this patch, but this appears to be wrong: we should do something like
> 
> if (!(value instanceof Enum<?> ev) || ev.getDeclaringClass() != clazz)
>     return SENTINEL; // or false

You are right, of course, but let's solve that under [JDK-8318144](https://bugs.openjdk.org/browse/JDK-8318144), to ensure that can more easily be backported. Tests are running on the fix, will publish PR once they pass. Thanks!

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 383:
> 
>> 381:               .withMethod("typeSwitch", MethodTypeDesc.ofDescriptor("(Ljava/lang/Object;ILjava/util/function/BiFunction;)I"), Classfile.ACC_STATIC, mb -> {
>> 382:                   mb.withFlags(AccessFlag.PUBLIC, AccessFlag.FINAL, AccessFlag.STATIC)
>> 383:                     .withCode(cb -> {
> 
> Can just use `clb.withMethodBody` and pass the access flags with `Classfile.ACC_PUBLIC | Classfile.ACC_STATIC | Classfile.ACC_FINAL`

Done.

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 432:
> 
>> 430:                                 cb.aload(2);
>> 431:                                 cb.constantInstruction(enumIdx);
>> 432:                                 cb.invokestatic(Integer.class.describeConstable().get(),
> 
> You can use `ConstantDescs.CD_Integer` etc.

Done.

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 434:
> 
>> 432:                                 cb.invokestatic(Integer.class.describeConstable().get(),
>> 433:                                                 "valueOf",
>> 434:                                                 MethodType.methodType(Integer.class,
> 
> I recommend `MethodTypeDesc.of()` instead.

Done.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381868349
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381871678
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381866683
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381866943
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381867739