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