[PATCH] minor regex cleanup: use switch for enum

[Remi Forax]

And as part of Amber, we are likely change the bytecode translation of the switch on enums (in fact all switches apart the one on integers) to avoid the separate compilation issues you mention (and support more kind of switches). The idea is that the new translation is to use invokedynamic so the association between an enum and the corresponding case is done at runtime and not at compile time.

I think there is already codes in the amber repository that deals with enums, so another way to optimize the switch on enum is to change the bootstrap method associated to the switch on enum (we may also have to introduce a new method handle combinator that works like the getter but with the @Stable semantics).

Rémi 

----- Mail original -----
> De: "Peter Levart" <peter.levart at gmail.com>
> À: "David Lloyd" <david.lloyd at redhat.com>, "Xueming Shen" <xueming.shen at oracle.com>
> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mardi 24 Avril 2018 14:17:21
> Objet: Re: [PATCH] minor regex cleanup: use switch for enum

> On 04/24/2018 12:41 PM, Peter Levart wrote:
>> public class Code {l
>>
>>     static class m$switch$1 {
>>         static final int[] caseindex = new int[X.values().length];
>>         static {
>>             caseindex[X.A.ordinal()] = 1;
>>             caseindex[X.B.ordinal()] = 2;
>>             caseindex[X.C.ordinal()] = 3;
>>         }
>>     }
>>
>>     public void m(X x) {
>>         switch (m$switch$1.caseindex[x.ordinal()]) {
>>             case 1: // ... branch A
>>                 break;
>>             case 2: // ... branch B
>>                 break;
>>             case 3: // ... branch C
>>                 break;
>>             default: // ... default branch        }
>>     }
>> }
>>
>>
>> Pefrormance-wise this is similar to switch on int. So pretty optimal.
>> Decision should only be made on the count of code clarity here.
>>
>> While speaking of performance of enum switches, I have a question for
>> a more knowledgeable person...
>>
>> Should JIT be able to fold above 'x' variable/parameter into a
>> constant (suppose m() was called in a loop with an explicitly
>> specified constant value such as X.A), it could further expand this
>> decision to the x.ordinal() value (if the final instance 'ordinal'
>> field in the X enum class was marked with @Stable), it could then
>> further expand this decision to the looked up value of the
>> m$switch$1.caseindex[] slot if caseindex array field in sytnhetic
>> class was marked with @Stable, therefore transforming the switch to a
>> switch on int constant, optimizing the JITed code to directly execute
>> branch A and eliminate all other branches from generated code.
>>
>> The question is whether JIT is presently treating those fields (and
>> array) as @Stable or not?
> 
> It seems not. Here's a benchmark:
> 
> http://cr.openjdk.java.net/~plevart/misc/SwitchBench/SwitchBench.java
> 
> 
> See enumConstSwitch between "Original JDK 10" and "JDK 10 + @Stable
> Enum.ordinal field".
> 
> Interesting thing is that JDK 9 seems to be better at JITing code than
> JDK 10 in these tests. Why is that?
> 
> I included results for Graal JIT compiler which shows that it probably
> does not contain support for @Stable annotation.
> 
> 
> The relevant part for this "minor regex cleanup" patch is the following:
> 
> SwitchBench.enumConstIf      avgt   10  2.125 ± 0.002  ns/op
> SwitchBench.enumConstSwitch  avgt   10  2.501 ± 0.025 ns/op
> SwitchBench.enumVarIf        avgt   10  2.593 ± 0.004 ns/op
> SwitchBench.enumVarSwitch    avgt   10  2.862 ± 0.053 ns/op
> 
> which indicates that "switch" is a little slower, but with @Stable
> Enum.ordinal field, it is on par with "if" at least for constant folded
> switched on values:
> 
> SwitchBench.enumConstIf      avgt   10  2.126 ± 0.007 ns/op
> SwitchBench.enumConstSwitch  avgt   10  2.159 ± 0.003 ns/op
> SwitchBench.enumVarIf        avgt   10  2.593 ± 0.004 ns/op
> SwitchBench.enumVarSwitch    avgt   10  2.842 ± 0.006 ns/op
> 
> 
> Regards, Peter