RFR: JDK-8219412: Eager enum class initialization with enum switch

Jan Lahoda jan.lahoda at oracle.com
Mon Apr 8 08:17:05 UTC 2019

Hi Remi,

Even though I understand what you mean, not sure if it is appropriate to 
do this here (i.e. under an ordinary P3 bug). If we only had the 
switch-over-enum, then it might be OK. But we even currently have the 
switch-over-string, and foresee a possibility of more complex switches, 
and this switch-over-enum bootstrap may the outlier among the other 
bootstraps (or maybe not; or maybe that is OK; but without a more 
general consideration, it is difficult to tell).

I'd be much more inclined to either:
-use a local solution that is not much different from the current state 
(i.e. version .00 or possibly .01 of this patch), and convert to the 
switch bootstrap when that's done
-work on the complete switch bootstraps effort, and accept it may take 
some time before this bug is fixed.


On 5.4.2019 23:23, forax at univ-mlv.fr wrote:
> ----- Mail original -----
>> De: "jan lahoda" <jan.lahoda at oracle.com>
>> À: "Remi Forax" <forax at univ-mlv.fr>
>> Cc: "compiler-dev" <compiler-dev at openjdk.java.net>, "Liam Miller-Cushon" <cushon at google.com>
>> Envoyé: Vendredi 5 Avril 2019 19:35:35
>> Objet: Re: RFR: JDK-8219412: Eager enum class initialization with enum switch
>> On 5.4.2019 19:14, forax at univ-mlv.fr wrote:
>>> ----- Mail original -----
>>>> De: "jan lahoda" <jan.lahoda at oracle.com>
>>>> À: "Remi Forax" <forax at univ-mlv.fr>
>>>> Cc: "compiler-dev" <compiler-dev at openjdk.java.net>, "Liam Miller-Cushon"
>>>> <cushon at google.com>
>>>> Envoyé: Vendredi 5 Avril 2019 13:56:58
>>>> Objet: Re: RFR: JDK-8219412: Eager enum class initialization with enum switch
>>>> On 4.4.2019 20:39, Remi Forax wrote:
>>>>> Hi Jan,
>>>>> it seems like a perfect case for a constant dynamic,
>>>>> the array can be loaded using ldc and the bootstrap method takes the
>>>>> enum names as constant arguments.
>>>> True, I didn't realize that. javac does not currently (AFAIK) have a way
>>>> to generate condy, so we first need support for condy in javac. This
>>>> patch strives to do that:
>>>> http://cr.openjdk.java.net/~jlahoda/8219412/webrev.condy.01/
>>>> based on Vicente's:
>>>> http://hg.openjdk.java.net/amber/amber/rev/8e2f2d280e44
>>>> The usage to use that to generate the condy where available (and fall
>>>> back on generating the holder classes where not available):
>>>> http://cr.openjdk.java.net/~jlahoda/8219412/webrev.01/
>>> The description of an enum (EnumMapping) can be encoded by the enum class and an
>>> array of String (in the order of the enum values at compile time).
>>> So instead of generating one specific method per enum that initializes the
>>> mapping table and using it as a bootstrap method, i believe it's better to have
>>> one bootstrap method in the JDK (let say in
>>> java.lang.invoke.ConstantBootstraps) that takes the enum class and an array of
>>> Strings. This method will use ConstantBootstraps.enumConstant to get the
>>> corresponding enum value at runtime (which does the security checks) and a call
>>> ordinal() on each enum value in order to dynamically create the mapping table.
>>> This will reduce the size of the generated bytecode by removing the need to have
>>> one bootstrap method per enum.
>> Well, this is effectively the "use condy for switch" effort, right?
> yes !
>> (The enumSwitch method from:
>> http://hg.openjdk.java.net/amber/amber/file/4461b14e6543/src/java.base/share/classes/java/lang/compiler/SwitchBootstraps.java
>> is relevant, as is stringSwitch for switch-over-strings.) Not sure if we
>> have time for that for 13.
> The switch-over-enum proposed by SwitchBootstraps uses an invokedynamic and understanding the perf of indy is not that simple.
> The code you are proposing uses condy, i think we should keep it that way.
> At the same time, your patch generates bytecodes to initialize the array. I think this is an overkill.
> As shown by SwitchBootstraps.enumSwitch, you can represent the information that the compiler see at compile time with an enum class and an array of enum value names,
> so what i'm proposing is to still use condy to load the array of enum constants but to only use one bootstrap method that dynamically create the array.
> For the compiler, it's easier because you generate less bytecodes (it's not fully true because it seems the compiler generate the EnumMapping pretty early),
> in term of bugs, because you always call the same bootstrap method which is inside the JDK, if there is a bug, you can fix it without changing neither the compiler not the generated code,
> and it makes the .class slimmer which is usually a win.
> so the idea is to still use condy but initialize it by sending a call and an array of strings instead of initialize it by a special tailored bootstrap method.
> to be 100% clear, here is the BSM i think we should use:
>    public static int initEnumMappingArray(Lookup lookup,
>                                           String name,
>                                           Class<?> type,     // should be int[].class
>                                           Class<?> enumClass,
>                                           String...  enumNames) {
>      requireNonNull(lookup);
>      requireNonNull(name);
>      requireNonNull(type);
>      validateClassAccess(lookup, enumClass);   // does the class that contains the ldc condy can see the array?
>      int length = enumClass.getEnumConstants().length;
>      int[] array = new int[length];
>      for(int i = 0; i < enumNames.length; i++) {
>        try {
>          Enum<?> enumValue = Enum.valueOf(enumClass, enumNames[i]);
>          array[enumValue.ordinal()] = i + 1;
>        } catch (IllegalArgumentException e) {
>          // empty
>        }
>      }
>      return array;
>    }
> This is the same code as the code which is generated currently by the compiler,
> it can be optimized using java.lang.Class internals but this can be done later
>> Jan
> Rémi
>>> Also in Target.java, constant dynamic is available since 11 but if there is a
>>> need of a new BSM in ConstantBootstraps, it should be since 13 (apart if the
>>> patch is backported to 11).
>>>> (The downside of that is that when indy switch desugaring will be
>>>> implemented, there will be three distinct desugarings for switches over
>>>> enums.)
>>> yes, as you said, we may able to share some code in the future, we will see when
>>> we get there.
>>>>> You doesn't have to create one class per switch, it's lazy initialized
>>>> Please note that neither the original patch, nor the current javac
>>>> state, creates a class per switch. It creates a field (and with the .00
>>>> version of the patch a class) per enum used as switch selector per
>>>> top-level class.
>>> yes
>>>> Jan
>>> Rémi
>>>>> like any constant pool item and each switch on the same enum type can
>>>>> reuse the same,
>>>>> compare to the solution implemented by Eclipse, it's not a field but a
>>>>> constant pool constant you you can not mess with it using reflection and
>>>>> you don't need to check if the array was loaded or not in the fastpath.
>>>>> regards,
>>>>> Rémi
>>>>> ------------------------------------------------------------------------
>>>>>       *De: *"Liam Miller-Cushon" <cushon at google.com>
>>>>>       *À: *"jan lahoda" <jan.lahoda at oracle.com>
>>>>>       *Cc: *"compiler-dev" <compiler-dev at openjdk.java.net>
>>>>>       *Envoyé: *Jeudi 4 Avril 2019 20:20:00
>>>>>       *Objet: *Re: RFR: JDK-8219412: Eager enum class initialization with
>>>>>       enum switch
>>>>>       Hi Jan,
>>>>>       There are a couple of other bugs about enum switch / class
>>>>>       initialization issues that I think are addressed by this approach:
>>>>>       https://bugs.openjdk.java.net/browse/JDK-7176515
>>>>>       https://bugs.openjdk.java.net/browse/JDK-8169526
>>>>>       Do you have a sense of how many additional auxiliary classes this
>>>>>       will create for typical applications? I'm wondering if the number of
>>>>>       additional classes might be large enough to effect performance (i.e.
>>>>>       binary size, classloading).
>>>>>       Were other lazy initialization patterns considered? JDK-7176515
>>>>>       mentions that ecj avoids this by caching the switch map in a field
>>>>>       in the same class as the switch.
>>>>>       On Thu, Apr 4, 2019 at 11:04 AM Jan Lahoda <jan.lahoda at oracle.com
>>>>>       <mailto:jan.lahoda at oracle.com>> wrote:
>>>>>           Hi,
>>>>>           When there is a switch over enum in the code, and is compiled
>>>>>           into the
>>>>>           bytecode, the meaning of the switch should not change even if
>>>>>           the enum's
>>>>>           constant are reordered. This is achieved by a map that maps the
>>>>>           (current) enum constants to the appropriate case in the switch.
>>>>>           This map
>>>>>           is stored in a field in an auxiliary class, and filled when that
>>>>>           auxiliary class is loaded in.
>>>>>           But, if there are multiple switches over distinct enums under a
>>>>>           single
>>>>>           top-level class, the mapping fields for all the enums are stored
>>>>>           in a
>>>>>           single class. So, when any of these is needed, the mapping is
>>>>>           constructed for all the enums, which may cause enum class
>>>>>           initialization
>>>>>           even in cases where it shouldn't happen.
>>>>>           The proposed fix is to create a separate auxiliary classes for each
>>>>>           enum/mapping.
>>>>>           Proposed patch:
>>>>>           http://cr.openjdk.java.net/~jlahoda/8219412/webrev.00/
>>>>>           JBS: https://bugs.openjdk.java.net/browse/JDK-8219412
>>>>>           How does this look?
>>>>>           Thanks,
>>>>>                 Jan

More information about the compiler-dev mailing list