RFR: 8291769: Translation of switch with record patterns could be improved [v7]

[Jan Lahoda]

On Fri, 30 Sep 2022 12:21:58 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This is an attempt to improve the performance and scalability of switches with record patterns.
>> 
>> There are two main parts of this patch:
>> 1. for cases of consecutive runs of cases with the same record pattern, these are replaced with a single case and a nested switch. E.g.:
>> 
>> switch (obj) {
>>      case Box(String s) -> {}
>>      case Box(Integer i) -> {}
>>      case Box(Number n) -> {}
>>      ...
>> }
>> =>
>> switch (obj) {
>>      case Box b ->
>>           switch (b.o()) {
>>               case String s -> {}
>>               case Integer i -> {}
>>               case Number n -> {}
>>               default -> continue-with-outer-switch;
>>           };
>>     ...
>> }
>> 
>> 
>> This is done by first unrolling the record patterns into simple binding patterns a guards, and then by finding cases with the common binding pattern as a label and a viable guard. Binding patterns are reused as much as possibly, eliminating specialized handling of record patterns as much as possible.
>> 
>> 2. When a record accessor method fails with an exception, we need to wrap this exception with a `MatchException`. Currently this is being done by introducing a special proxy method which catches the exception and re-throws. The proposed patch here eliminates the need for the accessor methods by producing an appropriate `ExceptionTable` in the classfile, and a separate catch handler. This handler is attached to the innermost usable block, which is either the method block, lambda body, (static or non-static) initializer or a try block. This should ensure correct semantics, while not producing too many unnecessary catch handlers.
>> 
>> I ran the new code through a JMH benchmark:
>> [PatternsOptimizationTest.java.txt](https://github.com/openjdk/jdk/files/9260707/PatternsOptimizationTest.java.txt)
>> 
>> The results are:
>>  - for "long" testcase (a switch with many cases):
>> 
>> PatternsOptimizationTest.testExistingTranslationLongSwitch   thrpt   25  1025740.668 ± 15325.355  ops/s
>> PatternsOptimizationTest.testNewTranslationLongSwitch        thrpt   25  1588461.471 ± 15315.509  ops/s
>> 
>>  - for "short" testcase (a switch with no so many cases):
>> 
>> PatternsOptimizationTest.testExistingTranslationShortSwitch  thrpt   25  6418845.624 ± 75981.939  ops/s
>> PatternsOptimizationTest.testNewTranslationShortSwitch       thrpt   25  6894823.439 ± 67420.858  ops/s
>> 
>> 
>> So, the performance seems to be improved, at least in these cases.
>> 
>> As a follow-up work, there are several other improvements that may be worth investigating like using if cascades instead of switches with very few cases (but possibly not very trivial for switch expressions, at least for switch expressions of type `boolean`), or improving the code produced by the runtime bootstrap method (`SwitchBootstraps.typeSwitch`).
>
> Jan Lahoda has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Reflecting review feedback.
>  - Merge branch 'master' into JDK-8291769
>  - Merge branch 'master' into JDK-8291769
>  - Merge branch 'master' into JDK-8291769
>  - Reflecting review feedback.
>  - Merge branch 'master' into JDK-8291769
>  - Using a custom record instead of a generic Pair.
>  - Fixing test.
>  - Properly handle null record components.
>  - 8291769: Translation of switch with record patterns could be improved

Thanks for all the comments! I've reflected the feedback here:
https://github.com/openjdk/jdk/pull/9746/commits/4f9f7f9859c98f6312675455bdfa02f02b0015c7

and added additional comments.

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

PR: https://git.openjdk.org/jdk/pull/9746