8332522: SwitchBootstraps::mappedEnumLookup constructs unused array

[lahodaj]

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.

---

Progress

• [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue

Issue

• JDK-8332522: SwitchBootstraps::mappedEnumLookup constructs unused array (Bug - P4)

Reviewers

• Claes Redestad (@cl4es - Reviewer) ⚠️ Review applies to 8ab9ee64
• Chen Liang (@liach - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19906/head:pull/19906
$ git checkout pull/19906

Update a local copy of the PR:
$ git checkout pull/19906
$ git pull https://git.openjdk.org/jdk.git pull/19906/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19906

View PR using the GUI difftool:
$ git pr show -t 19906

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19906.diff

Webrev

Link to Webrev Comment