RFR 8200261: Regression with JVM anonymous class

[harold seigel]

Hi Lois,

Thanks for the review.

I'd prefer to set _this_class_index for all InstanceKlass objects 
because I don't think it is a good idea for fields in InstanceKlass 
objects to only be initialized some of the time. Also, there is no 
significant cost to the initialization. Although the field is currently 
only used for anonymous classes, perhaps there will be other uses for it 
in the future.

Thanks, Harold


On 4/4/2018 4:50 PM, Lois Foltan wrote:
> On 4/4/2018 1:29 PM, harold seigel wrote:
>
>> Hi,
>>
>> Please review this JDK-11 fix for JDK-8200261.  When the JVM default 
>> method processing creates an expanded constant pool, to add overpass 
>> methods, for an anonymous class, it needs to ensure that the 
>> (eagerly) resolved anonymous class get copied into the new constant 
>> pool's klasses array.  Otherwise, the anonymous class will be unable 
>> to find its own fields and methods.  This is only needed for 
>> anonymous classes because other classes get resolved after default 
>> method processing.
>>
>> Open Webrev: 
>> http://cr.openjdk.java.net/~hseigel/bug_8200261/webrev/index.html
>>
>> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8200261
>
> Hi Harold,
> Fix looks good.  Two minor clarification questions about the setting & 
> retrieving of the index, since this is intended only for anonymous 
> classes:
>
> share/classfile/classFileParser.cpp:
> - line #5427 - did you consider calling set_this_class_index from 
> within the "if (is_anonymous())" if statement at line #5429
> share/oops/instanceKlass.hpp:
> - line #521 - should there be asserts in these setter/getter methods 
> to check for an anonymous class?
>
> Thanks,
> Lois
>
>>
>> This fix was tested with Mach5 tiers 1 and 2 tests and builds on all 
>> Mach5 platforms, with tiers 3-5 tests on Linux-x64, with JCK-10 Lang 
>> and VM tests, and with additional JDK JTReg tests.
>>
>> Thanks, Harold
>>
>