[PATCH] minor regex cleanup: use switch for enum

[Remi Forax]

Hi Peter,
instead of using @Stable, John Rose had an interesting idea, we talk about a similar issue at last FOSDEM, make all final fields in java.lang trusted by default. 
The idea is that classes in java.lang doesn't seems to use Unsafe when deserializing (like classes in java.util does), so if this is true, all fields of java.lang should be trusted by the VM.

regards,
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