Soliciting opinions on JDK-8219412

[Maurizio Cimadamore]

I agree with Brian. If we want to fix this and put our hands in the enum 
switch classification, I think the best use of our time would be to 
tweak enum switches to adopt the superior indy-based classification 
approach, which should be immune to the issue you present.

Thanks
Maurizio

On 09/01/2023 15:35, Brian Goetz wrote:
> Another strategy is to use an `invokedynamic` as a switch classifier. 
>  (I believe there is even a bootstrap for this implemented somewhere.)
>
> The idea is that we lower a switch
>
>     switch (e) [
>         case A: …
>         case B: …
>         …
>     }
>
> Into a compact switch on int, and use an indy to map the target to a 
> case number:
>
>     switch (indy [BSM=EnumClassifier, type=(E)I, args = [ E.class, 
> “A”, “B”, … ]) {
>         case 1: …
>         Case 2: …
>     }
>
>  The indy bootstrap takes as a dynamic argument the switch operand 
> (enum value), and returns an int corresponding the case number. 
>  The static argument list identifies the enum constants (by string) 
> corresponding to the case numbers.  The bootstrap builds the array 
> that we would have built at compile time, curries it onto a method 
> handle, and wraps the method handle in a ConstantCallSite.  Then the 
> array is only built the first time we invoke the switch.  Thereafter 
> we get similar behavior to the current scheme, but without static 
> initializers.
>
>
>
>
>
>> On Jan 9, 2023, at 10:02 AM, Archie Cobbs <archie.cobbs at gmail.com> wrote:
>>
>> I'm looking for advice/opinions on the best way to 
>> address JDK-8219412 - Eager enum class initialization with enum switch.
>>
>> The problem stems from how the compiler implements switches on enum. 
>> The obvious simple approach is to get the enum's ordinal() value and 
>> then revert to a normal integer value switch. However, this snapshots 
>> the ordinal values at compile-time, and thus can fail if 
>> the Enum class is recompiled later.
>> Instead the compiler uses a lookup table. This table is dynamically 
>> constructed at runtime by a static initializer in a synthetic class. 
>> When compiling Foo.java, a separate lookup table is created for each 
>> enum type that is switched on anywhere in Foo.java. These lookup 
>> tables are all put into a single synthetic class Foo$1 and are all 
>> initialized at the same time, in a single static initializer.
>>
>> Now suppose Foo contains switches on enum types E1, E2, and E3 and a 
>> switch on E1 is encountered. This will cause classes E2 and E3 to be 
>> initialized, even though there is no 'first use' of either class. 
>> This is the bug.
>>
>> Strategy #1 - Put each lookup table into a separate synthetic class.
>>
>> You can see the reverse of this fix in this diff.
>>
>> The upside of this approach is that it fixes the bug without changing 
>> the approach and therefore presumably minimally affecting performance.
>>
>> The downside is an increase in the number of synthetic classes 
>> created: for every enum type switched on in Foo.java but declared in 
>> some other source file, there is a synthetic class 
>> created: Foo$1, Foo$2, etc. Is that a show-stopper??
>>
>> The total amount of code generated is basically the same, it's just 
>> spread over multiple class files instead of one.
>>
>> Strategy #2 - Switch on String identifiers instead of ordinal values
>>
>> This approach is straightforward, however the performance suffers a 
>> good bit in my tests. Probably not an option.
>>
>> Strategy #3 - Don't rely on a static initializer to build the mapping 
>> tables
>>
>> This approach would mean not relying on class initialization to 
>> trigger building the mapping tables, so we could build each table 
>> separately on demand. The synthetic class would look something like this:
>>
>> synthetic /* volatile ? */ int[] $EnumMap$E1;
>> synthetic /* volatile ? */ int[] $EnumMap$E2;
>> synthetic /* volatile ? */ int[] $EnumMap$E3;
>>
>> static int[] getEnumMap$E1() {
>>     if ($EnumMap$E1 == null) {
>>         /* build map here */
>>     }
>>     return $EnumMap$E1;
>> }
>>
>> static int[] getEnumMap$E2() {
>>     if ($EnumMap$E2 == null) {
>>         /* build map here */
>>     }
>>     return $EnumMap$E2;
>> }
>>
>> static int[] getEnumMap$E3() {
>>     if ($EnumMap$E3 == null) {
>>         /* build map here */
>>     }
>>     return $EnumMap$E3;
>> }
>>
>> Strategy #4 - Other ideas?
>>
>> Thanks,
>> -Archie
>>
>> -- 
>> Archie L. Cobbs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/compiler-dev/attachments/20230109/b313cc5d/attachment.htm>