[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR (M) 8046246: the constantPoolCacheOopDesc::adjust_method_entries()
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2015-02-20 19:19:03
Message-ID: 54E788A7.1000103 () oracle ! com
[Download RAW message or body]

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@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
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]