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

[Jan Lahoda]

On Fri, 3 Nov 2023 15:50:43 GMT, Rémi Forax <forax 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 429:
> 
>> 427:                         Label next = element.next();
>> 428:                         cb.labelBinding(element.target());
>> 429:                         if (element.caseLabel() instanceof Class<?> classLabel &&
> 
> I think you can do a if else of isPresent inside instanceof Class<?> to avoid to reapeat the instanceof and store `classLabel.describeConstable()` into a local variable.

Done, thanks.

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 437:
> 
>> 435:                             cb.aload(3);
>> 436:                             cb.constantInstruction(extraClassLabels.size());
>> 437:                             cb.aaload();
> 
> Arrays are mutable in Java, so the VM can not know if the array of non denotable classes (`extraClassLabels`) will be changed or not so the result of aaload is not a constant so the call to isInstance can not be optimized. Using a immutable list (`List.of()`) instead of an array should work, because all the implementation of List.of() are using @Stable. In that case aaload becomes invokevirtual List.get().

Done, thanks.

> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 517:
> 
>> 515:                                                                               BiPredicate.class,
>> 516:                                                                               Class[].class));
>> 517:             return MethodHandles.insertArguments(typeSwitch, 2, new ResolvedEnumLabels(caller, enumDescs.toArray(s -> new EnumDesc<?>[s])),
> 
> you can use the method reference `EnumDesc[]::new`instead of `s -> new EnumDesc<?>[s]` and same below Class[]::new  (the wirldcard should not be necessary)

Done, thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963834
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963986
PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963719