On Fri, 3 Nov 2023 15:32:34 GMT, Jan Lahoda <jlah...@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). > > Jan Lahoda has updated the pull request incrementally with two additional > commits since the last revision: > > - Some more get->orElseThrow > - Reflecting review feedback. src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 385: > 383: > 384: byte[] classBytes = > Classfile.of().build(ClassDesc.of(typeSwitchClassName(caller.lookupClass())), > clb -> { > 385: clb.withFlags(AccessFlag.FINAL, AccessFlag.SYNTHETIC) `AccessFlag.SUPER` is missing, this will make this class a value class in the Valhalla repo ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381896567