Serguei,
Good find!! The fix looks good. I'm sure the optimization wasn't
noticeable and thank you for the additional comments.
There is a Method::external_name() function that I believe prints all
the things you want in the logging here:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.1/src/hotspot/share/oops/cpCache.cpp.udiff.html
I don't need to see another webrev if you make this change.
Thanks,
Coleen
On 5/14/20 12:26 PM, serguei.spit...@oracle.com wrote:
Please, review a fix for The Kitchensink bug:
https://bugs.openjdk.java.net/browse/JDK-8222005
Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.1/
Summary:
The VM_RedefineClasses::doit()uses two helper classes to walk all VM
classes.
First is AdjustAndCleanMetadata to adjust method entries in the
vtables/itables/cpcaches.
Second is CheckClass to check that adjustments for all method
entries are correct.
The Kitchensink test is failing with two modes:
- guarantee(false) failed: OLD and/or OBSOLETE method(s) found in the
VM_RedefineClasses::CheckClass::do_klass()
- SIGSEGV in the
ConstantPoolCacheEntry::get_interesting_method_entry() in context
of VM_RedefineClasses::CheckClass::do_klass() execution
The second failure mode is rare. In is before the first one in the
code path.
The root cause of both is that the
VM_RedefineClasses::AdjustAndCleanMetadata::do_klass()
is skipping the cpcache update for classes that are being redefined
assuming they are
being redefined by the current VM_RedefineClasses operation. In such
cases, the adjustment
is not needed as the cpcache is empty. The problem is that the
assumption above is wrong.
The class can also be redefined by another VM_RedefineClasses
operation which has already
executed its doit_prologue. The cpcache djustment for such class is
necessary.
The fix is to always call the cp_cache->adjust_method_entries() even
if the class is
being redefined by the current VM_RedefineClasses operation. It is
possible to skip it
but it will add extra complexity to the code.
The fix also includes minor tweak in the cpCache.cpp to include
method's class name to
the redefinition cpcache log.
Testing:
Ran Kitchensink test locally on a Linux server with the
Instrumentation module enabled.
The test does not fail anymore.
In progress, a mach5 tiers 1-5 and runs and separate mach5
Kitchensink run.
Thanks,
Serguei