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

[serguei.spit...@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.spit...@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