EnumData space optimization in j.l.Class (JEP-146)

[Remi Forax]

On 12/17/2012 11:14 PM, Peter Levart wrote:
> On 12/17/2012 10:26 PM, Mandy Chung wrote:
>> On 12/17/12 7:36 AM, Peter Levart wrote:
>>> Hi David and others,
>>>
>>> Here's a patch that eliminates one of two fields in java.lang.Class, 
>>> related to caching enum constants:
>>>
>>> http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html 
>>>
>>>
>>> It does it by moving one field to a subclass of HashMap, which is 
>>> referenced by a remaining field that serves two different 
>>> purposes/stages of caching.
>>>
>>
>> Your observation of merging the enumConstants and 
>> enumConstantDirectory is a good one.   I see that caching of 
>> enumConstantDirectory is important as it's used by EnumMap and 
>> EnumSet whose performance is critical (specified with constant time 
>> operations).  I'm unsure about Class.getEnumConstants whether it's 
>> performance critical and worths the complexity of your proposed fix 
>> (the enumData field of two types).  If a class has cached an 
>> enumConstantDirectory, Class.getEnumConstants can return a clone of 
>> its values().
>>
>> Anyone knows how Class.getEnumConstants is commonly used and needs to 
>> be performant?  I suspect it's more typical to obtain the list of 
>> enum constants statistically (calling Enum.values()) than reflectively.
> Hi Mandy,
>
> public Class.getEnumConstants() is a reflection mirror of 
> SomeEnum.values(). It returns a defensive copy of the constants array. 
> The primary place for Enum constants is in a private static final 
> $VALUES field, generated by compiler in each Enum subclass. But that I 
> think is not part of specification, so for internal usage (as far as I 
> have managed to find out only in the constructors of EnumSet and 
> EnumMap), the package-private Class.getEnumConstantsShared() is used 
> which obtains a copy of the array by calling SomeEnum.values() and 
> than caches is.
>
> The Class.enumConstantDirectory() on the other hand is an internal 
> package-private method that returns a shared/cached Map<String, T>, 
> which is used internally to implement SomeEnum.valueOf(String) and 
> Enum.valueOf(Class, String) static methods.
>
> Both package-private methods must be fast.
>
> Regards, Peter

for what it worth, I'm the guy behind the patch of bug 6276988 (it was 
before OpenJDK was setup BTW),
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6276988
and for the little story, I need that patch because I was developing an 
Eclipse plugin that uses EnumSet to represent the possible completion 
values.
So to answer to Mandy, this application needs really fast EnumSet 
creation thus really fast getEnumConstantShared() because the EnumSets 
was created as user types code.

Also, Peter, in your getEnumConstantShared(), while the first instanceof 
is a cheap one, the second is not.
I think I prefer either the status quo or to group all exotic fields in 
a specific object and pay the indirection to that object but not the 
instanceof checks.

cheers,
Rémi

>
>>
>> Mandy
>>
>>> These are the results of a micro-benchmark that exercises public API 
>>> that uses the internal j.l.Class API regarding enum constants:
>>>
>>> enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, 
>>> TEN }
>>> EnumSet.noneOf(MyEnum.class): 300_000_000 loops
>>> MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names
>>>
>>> ** Original JDK8 code
>>>
>>> Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
>>> ../out/production/test test.EnumTest reference
>>>
>>>       EnumSet.noneOf(Class): 351610312 340302968 339893333 339774384 
>>> 339750612 339558414 339547022 339621595
>>>      MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
>>> 886119463 885818334 885827093 885752461
>>>       EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
>>> 339512154 339511634 339664326 339793144
>>>
>>> ** patched java.lang.Class
>>>
>>> Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
>>> ../out/production/test -Xbootclasspath/p:../out/production/jdk 
>>> test.EnumTest
>>>
>>>       EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
>>> 305058303 305044144 305073463 305049604
>>>      MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
>>> 891414312 893652469 891412757 891409294
>>>       EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
>>> 406765274 406815728 407002576 406779162
>>>
>>> The slow-down of about 20% (last line) is presumably a consequence 
>>> of another in-direction to obtain shared enum constants array when 
>>> there is already a Map in place. It is still fast though (300M 
>>> EnumSet instances / 0.4 s).
>>>
>>> Here's the source of the micro-benchmark:
>>>
>>> https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java 
>>>
>>>
>>> I don't know what's more important in this occasion. A small space 
>>> gain (8 or 4 bytes per j.l.Class instance) or a small performance 
>>> gain (20%).
>>>
>>> Regards, Peter
>>>
>>
>