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

[Peter Levart]

At end...

On 12/18/2012 07:50 PM, Remi Forax wrote:
> On 12/18/2012 10:18 AM, Peter Levart wrote:
>> On 12/17/2012 11:39 PM, Remi Forax wrote:
>>> 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.
>> Hi Rémi,
>>
>> is 600M EnumSets / sec good enough for a fast typist?
>
> The eclipse plugin uses is a LR parser (a GLR exactly) which has the 
> stupid property that it can change the whole parse tree if you just 
> add one character.
>
> Also i'm maybe wrong but, if you test with several different 'enum 
> classes', you should see a slowdown.
>
> Rémi
>
You're right. I modified the test to use 5 different enum classes 
interchangeably in a loop:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java

The slowdown of EnumSet.noneOf() compared to original code was more 
apparent - about 50% when there's an array assigned to enumData field 
and more than 60% when there's already an EnumData object holding the 
array reference. MyEnum.valueOf(String) seems to be unaffected - more 
cycles spent elsewhere in the code-path to be significant.

When I changed the "if (enumData instanceof Enum[])" with "if (enumData 
!= null)", the slowdown of EnumSet.noneOf() was less apparent - about 
24% when there's an array assigned to enumData and about 44% when 
there's EnumData object. MyEnum.valueOf(String) shows the same performance.

So in order to affect the performance as little as possible, I devised a 
more direct approach without instanceof checks and casts. Here it is:

http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.02/index.html

With this change, EnumSet.noneOf() is only about 12% slower than 
original code and MyEnum.valueOf(String) performs the same. This code 
has a space overhead of an extra object with two fields, but it is 
expected that the number of enum classes compared to non-enum classes is 
small and the reduction of a field in the java.lang.Class pays off. Even 
though this is a single field and it might not shrink (not every but 
approx. half of all?) java.lang.Class object (s) on 32bit architectures 
due to alignment constraints, it presents an opportunity for shrinkage 
when other individual fields in java.lang.Class are similarly removed. 
One such opportunity presents the 2+1 remaining fields used for caching 
annotations.

I understand that contributions to JEP-149 are closed already, but I 
post this here for any future reference.

Regards, Peter