RFR: 8332522: SwitchBootstraps::mappedEnumLookup constructs unused array

Jan Lahoda jlahoda at openjdk.org
Wed Jun 26 12:37:23 UTC 2024


For general pattern matching switches, the `SwitchBootstraps` class currently generates a cascade of `if`-like statements, computing the correct target case index for the given input.

There is one special case which permits a relatively easy faster handling, and that is when all the case labels case enum constants (but the switch is still a "pattern matching" switch, as tranditional enum switches do not go through `SwitchBootstraps`). Like:


enum E {A, B, C}
E e = ...;
switch (e) {
     case null -> {}
     case A a -> {}
     case C c -> {}
     case B b -> {}
}


We can create an array mapping the runtime ordinal to the appropriate case number, which is somewhat similar to what javac is doing for ordinary switches over enums.

The `SwitchBootstraps` class was trying to do so, when the restart index is zero, but failed to do so properly, so that code is not used (and does not actually work properly).

This patch is trying to fix that - when all the case labels are enum constants, an array mapping the runtime enum ordinals to the case number will be created (lazily), for restart index == 0. And this map will then be used to quickly produce results for the given input. E.g. for the case above, the mapping will be `{0 -> 0, 1 -> 2, 2 -> 1}` (meaning `{A -> 0, B -> 2, C -> 1}`).

When the restart index is != 0 (i.e. when there's a guard in the switch, and the guard returned `false`), the if cascade will be generated lazily and used, as in the general case. If it would turn out there are significant enum-only switches with guards/restart index != 0, we could improve there as well, by generating separate mappings for every (used) restart index.

I believe the current tests cover the code functionally sufficiently - see `SwitchBootstrapsTest.testEnums`. It is only that the tests do not (and regression tests cannot easily, I think) differentiate whether the special-case or generic implementation is used.

I've added a new microbenchmark attempting to demonstrate the difference. There are two benchmarks, both having only enum constants as case labels. One, `enumSwitchTraditional` is an "old" switch, desugared fully by javac, the other, `enumSwitchWithBootstrap` is an equivalent switch that uses the `SwitchBootstraps`. Before this patch, I was getting values like:

Benchmark                           Mode  Cnt   Score   Error  Units
SwitchEnum.enumSwitchTraditional    avgt   15  11.719 ± 0.333  ns/op
SwitchEnum.enumSwitchWithBootstrap  avgt   15  24.668 ± 1.037  ns/op


and with this patch:

Benchmark                           Mode  Cnt   Score   Error  Units
SwitchEnum.enumSwitchTraditional    avgt   15  11.550 ± 0.157  ns/op
SwitchEnum.enumSwitchWithBootstrap  avgt   15  13.225 ± 0.173  ns/op


So, this seems like a clear improvement to me.

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

Commit messages:
 - 8332522: SwitchBootstraps::mappedEnumLookup constructs unused array

Changes: https://git.openjdk.org/jdk/pull/19906/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19906&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332522
  Stats: 187 lines in 2 files changed: 149 ins; 18 del; 20 mod
  Patch: https://git.openjdk.org/jdk/pull/19906.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19906/head:pull/19906

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


More information about the core-libs-dev mailing list