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

[Rémi Forax]

On Fri, 3 Nov 2023 08:41:12 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider code like:
> 
> void test(Object o) {
>     switch (o) {
>         case X1 -> {}
>         case X2 -> {}
> ...(about 100 cases)
> ``` 
> 
> javac will compile the switch into a switch whose selector is an indy invocation to `SwitchBootstraps.typeSwitch`, with static arguments being the types in the cases.
> 
> `SwitchBootstraps.typeSwitch` will then create a chain of `MethodHandle`s performing `instanceof` checks between the switch's selector and the given case type. The problem is that when the number of cases is high enough, (more than ~40-50), the chain gets too long, and the tests won't inline anymore. This then leads to a very bad performance, when compared to manually written if-instanceof-else-if-instanceof- chain.
> 
> The proposal herein is to use bytecode (written using the ClassFile API/library) instead of the `MethodHandle`s chain. The overall performance of this seems to be similar to the manually written if-instanceof-else-if-instanceof- chain.
> 
> Using the benchmark from the bug, and this patch, I am getting:
> 
> MyBenchmark.testIfElse100  thrpt    5  521826.326 ± 7510.042  ops/s
> MyBenchmark.testSwitch100  thrpt    5  505440.170 ± 3757.178  ops/s
> 
> 
> The most tricky part of this new way to generate the tests is handling of non-type case labels, and in particular cases with enum constant labels. The resolution of enum constants is deferred as much as possible, by using an indirection through the `ResolvedEnumLabels`.
> 
> Further improvements may be possible, esp. for some specific cases (like all cases having a type, and the type being a final class).

Here is a test that uses a hidden class that works with the current implementation.
If i'm not mistaken, the proposed implementation fails that test. 


public class SwitchBootstrapTest {
  public static void main(String[] args) throws Throwable {
    var className = SwitchBootstrapTest.class.getName();
    byte[] data;
    try(var input = SwitchBootstrapTest.class.getResourceAsStream('/' + className.replace('.', '/') + ".class")) {
      data = input.readAllBytes();
    }

    var lookup = MethodHandles.lookup().defineHiddenClass(data, true, ClassOption.NESTMATE, ClassOption.STRONG);
    var hiddenClass = lookup.lookupClass();
    var constructor = lookup.findConstructor(hiddenClass, methodType(void.class));
    var instance = constructor.invoke();

    var methodType = methodType(int.class, Object.class, int.class);
    var callSite = SwitchBootstraps.typeSwitch(lookup, "", methodType, hiddenClass, Object.class);
    var index = (int) callSite.getTarget().invokeExact(instance, 0);
    System.out.println("index " + index);
  }
}

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

PR Comment: https://git.openjdk.org/jdk/pull/16489#issuecomment-1792629864