RFR: 8332522: SwitchBootstraps::mappedEnumLookup constructs unused array [v2]

Jan Lahoda jlahoda at openjdk.org
Thu Jun 27 14:06:42 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  ...

Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:

  Reflecting review feedback.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/19906/files
  - new: https://git.openjdk.org/jdk/pull/19906/files/8ab9ee64..79b3a76f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19906&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19906&range=00-01

  Stats: 17 lines in 1 file changed: 9 ins; 3 del; 5 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