RFR: JDK-8199406: Performance drop with Java JDK 1.8.0_162-b32

[Vladimir Kozlov]

Looks good.
You need other reviewers to look too.

Thanks,
Vladimir

On 3/29/18 3:14 PM, Poonam Parhar wrote:
> Hello Vladimir,
> 
> Thanks for reviewing the changes. Please find the updated webrev here:
> http://cr.openjdk.java.net/~poonam/8199406/webrev.01/
> 
> Regards,
> Poonam
> 
> On 3/29/2018 2:23 PM, Vladimir Kozlov wrote:
>> Looks good. I have few comments.
>>
>> You can remove old is_entry_point() which have the same code as new 
>> entry_point(). After changes it is used only in one place in 
>> CompiledIC::is_megamorphic(). It can be replace with NULL check of 
>> entry_point() result there. Add comment in vtableStubs.hpp for new 
>> method.
>>
>> is_metadata_method field name should start with _ (this is Hotspot 
>> convention).
>>
>> is_loader_alive() could be simplified since you have field now:
>>
>>   inline bool is_loader_alive(BoolObjectClosure* is_alive) {
>>     Klass* k = is_metadata_method ? 
>> ((Method*)_holder_metadata)->method_holder() : (Klass*)_holder_metadata;
>>     if (!k->is_loader_alive(is_alive)) {
>>       return false;
>>     }
>>     if (!_holder_klass->is_loader_alive(is_alive)) {
>>       return false;
>>     }
>>     return true;
>>   }
>>
>> Thanks,
>> Vladimir
>>
>> On 3/29/18 1:31 PM, Poonam Parhar wrote:
>>> Hello,
>>>
>>> Please review the changes for the following bug that improve the 
>>> nmethod unloading times with a couple of optimizations.
>>>
>>> JDK-8199406 <https://bugs.openjdk.java.net/browse/JDK-8199406>: 
>>> Performance drop with Java JDK 1.8.0_162-b32
>>> Webrev: http://cr.openjdk.java.net/~poonam/8199406/webrev.00/
>>>
>>> This changeset includes two changes:
>>> 1. In /compiledIC.cpp, CompiledIC::is_icholder_entry()/ , we need to 
>>> determine if the code blob is an itable stub. With this change, 
>>> before linearly searching through all the VtableStub entries, we 
>>> first check whether the codeblob is a vtable or not. We now also 
>>> parse through the list entries only once rather than doing it twice 
>>> in /VtableStubs::is_entry_point()/ and /VtableStubs::stub_containing()/.
>>> 2. The second change helps avoid the virtual function calls in 
>>> CompiledICHolder::is_loader_alive(). CompiledICHolder now stores 
>>> information whether the metadata it holds is a method or a klass.
>>>
>>> Testing:
>>> - Customer testing confirming that their class-unloading times drop 
>>> from 10s of seconds to an average of 0.75 secs.
>>> - mach5 jdk-tier1,jdk-tier2,jdk-tier3,hs-tier1,hs-tier2
>>>
>>> Thanks,
>>> Poonam
>>>
>