RFR (M) 8046246: the constantPoolCacheOopDesc::adjust_method_entries() used in RedefineClasses does not scale

[serguei.spitsyn at oracle.com]

Hi Coleen,

On 2/20/15 9:56 AM, Coleen Phillimore wrote:
>
> Hi Serguei,  This looks good but I have some comments:

Thanks a lot for good suggestions and help in the internal review!

>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8046246-JVMTI-redefscale.1/src/share/vm/oops/klassVtable.cpp.udiff.html 
>
>
> How can (old_method == new_method) in adjust_method_entries?   If 
> old_method->is_old() then you're not updating the vtables or the 
> itable?  It should assert with check_no_old_or_obsolete_entries() at 
> the end.


Nice catch, fixed in all 4 places.


>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8046246-JVMTI-redefscale.1/src/share/vm/oops/cpCache.cpp.udiff.html 
>
>
> It looks like is_interesting_method_entry and 
> get_interesting_method_entry are the same code only one returns bool 
> and the other returns the Method*.   Can you call 
> is_interesting_method_entry and check for null return so there are not 
> two copies of this code?

Nice catch as well, fixed.


I'll post the updated webrev after some testing.


Thanks,
Serguei

>
> thanks,
> Coleen
>
> On 2/19/15, 12:45 AM, serguei.spitsyn at oracle.com wrote:
>> Please, review the fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-8046246
>>
>>
>> Open hotspot webrevs:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8046246-JVMTI-redefscale.1/ 
>>
>>
>> Open jdk (new unit test) webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8046246-JVMTI-manymethods.1/ 
>>
>>
>>
>> Summary:
>>
>>    This performance/scalability issue in class redefinition was 
>> reported by HP and the Enterprise Manager team.
>>    The following variants of the adjust_method_entries() functions do 
>> not scale:
>>      ConstantPoolCache::adjust_method_entries()
>>      klassVtable::adjust_method_entries()
>>      klassItable::adjust_method_entries()
>>      InstanceKlass::adjust_default_methods()
>>
>>    The ConstantPoolCache::adjust_method_entries() is the most important.
>>
>>    The approach is to use the holder->method_with_idnum() like this:
>>      Method* new_method = 
>> holder->method_with_idnum(old_method->orig_method_idnum());
>>      if (old_method != new_method) {
>>          <replace old_method with new_method>
>>      }
>>
>>      New algorithm has effectiveness O(M) instead of original O(M^2),
>>      where M is count of methods in the class.
>>      The new test (see webrev above) was used to mesure CPU time 
>> consumed by the
>>      ConstantPoolCache::adjust_method_entries() in both original and 
>> new approach.
>>
>>      The performance numbers are:
>>
>> --------------------------------------------------------------------------------------- 
>>
>>      Methods: ------ 1,000 --------------- 10,000 ----------------- 
>> 20,000
>> --------------------------------------------------------------------------------------- 
>>
>>      Orig:         600,000 nsec (1x)   60,500,000 nsec (~100x) 
>> 243,000,000 nsec (~400x)
>>      New:           16,000 nsec (1x)      178,000 nsec (~10x) 355,000 
>> nsec  (~20x)
>> --------------------------------------------------------------------------------------- 
>>
>>
>>
>>
>> Testing:
>>   In progress: VM SQE RedefineClasses tests, JTREG 
>> java/lang/instrument, com/sun/jdi tests
>>
>>
>> Thanks,
>> Serguei
>>
>