On Mon, 29 May 2023 07:25:26 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

>> The pattern matching switches are using a bootstrap method 
>> `SwitchBootstrap.typeSwitch` to implement the jumps in the switch. 
>> Basically, for a switch like:
>> 
>> switch (obj) {
>>     case String s when s.isEmpty() -> {}
>>     case String s -> {}
>>     case CharSequence cs -> {}
>>     ...
>> }
>> 
>> 
>> this method will produce a MethodHandle that will be analyze the provided 
>> selector value (`obj` in the example), and will return the case index to 
>> which the switch should jump. This method also accepts a (re)start index for 
>> the search, which is used to handle guards. For example, if the 
>> `s.isEmpty()` guard in the above sample returns false, the matching is 
>> restarted on the next case.
>> 
>> The current implementation is fairly slow, it basically goes through the 
>> labels in a loop. The proposal here is to replace that with a MethodHandle 
>> structure like this:
>> 
>> obj == null ? -1
>>                   : switch (restartIndex) {
>>                         case 0 -> obj instanceof String ? 0 : obj instanceof 
>> CharSequence ? 2 : ... ;
>>                         case 1 -> obj instanceof String ? 1 : obj instanceof 
>> CharSequence ? 2 : ... ;
>>                         case 2 -> obj instanceof CharSequence ? 2 : ... ;
>>                         ...
>>                         default -> <labels-count>;
>>                     }
>> 
>> 
>> This appear to run faster than the current implementation, using testcase 
>> similar to the one used for https://github.com/openjdk/jdk/pull/9746 , these 
>> are the results
>> 
>> PatternsOptimizationTest.testLegacyIndyLongSwitch   thrpt   25   1515989.562 
>> ± 32047.918  ops/s
>> PatternsOptimizationTest.testHandleIndyLongSwitch   thrpt   25   2630707.585 
>> ± 37202.210  ops/s
>> 
>> PatternsOptimizationTest.testLegacyIndyShortSwitch  thrpt   25   6789310.900 
>> ± 61921.636  ops/s
>> PatternsOptimizationTest.testHandleIndyShortSwitch  thrpt   25  10771729.464 
>> ± 69607.467  ops/s
>> 
>> 
>> The "LegacyIndy" is the current implementation, "HandleIndy" is the one 
>> proposed here. The translation in javac used is the one from #9746 in all 
>> cases.
>
> Jan Lahoda has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains six commits:
> 
>  - Reflecting review feedback.
>  - Merge branch 'master' into JDK-8291966
>  - Adding comments
>  - Improving performance
>  - Merge branch 'master' into JDK-8291966
>  - 8291966: SwitchBootstrap.typeSwitch could be faster

This patch is intended to eliminate some consecutive unnecessary tests like in 
case like:

switch (o) {
    case Runnable r when ... -> {}
    case Runnable r when ... -> {}
    case Runnable r when ... -> {}
    case Object o -> {}
}


If `o` is not a `Runnable`, the `instanceof` will only happen for the first 
case, and the rest will be skipped, as these tests could not pass. But (as a 
current limitation), if it is not a consecutive run, the duplicate `instanceof` 
checks will still happen.

I am quite sure there are ways to improve the bootstrap further, but might be 
better to have some (more) real-world examples to know what to optimize for.

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

PR Comment: https://git.openjdk.org/jdk/pull/9779#issuecomment-1570306708

Reply via email to