Review request 7109878: The instanceKlass EnclosingMethhod attribute fields can be folded into the _inner_class field

[Tom Rodriguez]

On Feb 21, 2012, at 10:41 AM, Jiangli Zhou wrote:

> The updated webrev is http://cr.openjdk.java.net/~jiangli/7109878/webrev.01/.

How does the math work out for whether this is a net win on space?  If you have a bunch of klasses with EnclosingMethod attributes but no inner classes then you are allocating an extra Java array just to store 2 u2s.  For code that makes heavy use of inner classes might this cause it to take more space, particularly in 64 bit where the header is larger?

Did you consider making the inner_classes and enclosing methods arrays be embedded in the variable sized portion of the instanceKlass, following the nonstatic_oop_maps?  That would save a pointer for every class as well as reducing the overhead for inner classes array itself.

In instanceKlass.cpp, you seem to be using EXCEPTION_MARK to get THREAD defined and that's not really its purpose.  You don't even need THREAD defined in that code you can just use the constructor that doesn't take a thread argument and it will fetch the current thread.

I think it might be worth hiding the inner_classes array behind an iterator class instead of allowing direct access to the array and modifying all the iterators to use the new bounds check logic.  It just seems like it's asking for bugs.  Hiding it like this would be necessary if the array were embedded in the klass.

You'll also need to modify the SA handle this.

tom

> 
> Thanks,
> 
> Jiangli
> 
> On 02/17/2012 06:02 PM, Jiangli Zhou wrote:
>> Hi Dean,
>> 
>> Thanks for catching that. Yes, it needs to be bigger than u2 since "length" is already u2. I'll fix it to use int.
>> 
>> Thanks!
>> 
>> Jiangli
>> 
>> On 02/17/2012 05:44 PM, Dean Long wrote:
>>> In parse_classfile_inner_classes_attribute():
>>> 
>>> !   u2 size = length * 4 + (parsed_enclosingmethod_attribute ? 2 : 0);
>>> 
>>> don't you need a size bigger than u2 for "size"?  I think any value of "length" greater than 0x3fffffff will
>>> cause an overflow.
>>> 
>>> dl
>>> 
>>> On 2/17/2012 3:42 PM, Jiangli Zhou wrote:
>>>> Hi,
>>>> 
>>>> Please review the changes that fold the instanceKlass::_enclosing_method_class_index and instanceKlass::_enclosing_method_method_index into the instanceKlass::_inner_classes array. The instanceKlass::_enclosing_method_class_index and instanceKlass::_enclosing_method_method_index are only used if the class contains the EnclosingMethod attribute (local class or anonymous class). For majority of the classes, these two fields (4byte total) are wasted. Folding them into the _inner_classes array and only allocating the space for them saves 4byte for most of the loaded classes. The enclosing method indexes are located at the end of the _inner_classes array when exist.
>>>> 
>>>> http://cr.openjdk.java.net/~jiangli/7109878/webrev.00/
>>>> 
>>>> Thanks,
>>>> 
>>>> Jiangli
>> 
>