JVM/TI code review request (XS and M) (7182152) (original) (raw)

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Wed Feb 6 12:22:59 PST 2013

Looks good.

Thanks, Serguei

On 2/6/13 11:54 AM, Karen Kinnear wrote:

Thank you Dan - this is much better and sets a good model for the rest of us.

thanks, Karen On Feb 6, 2013, at 9:05 AM, Daniel D. Daugherty wrote:

Adding other alias and people back onto the thread...

Thanks for the re-review!

On 2/6/13 6:41 AM, Coleen Phillimore wrote: This is good that you added the INCLUDEJVMTI. I didn't think it'd add that much space, but it a good change. I didn't think it would add much space either, but... It gave me a chance to check out the MinimalVM stuff a bit and it serves to identify those pieces of code as being associated with JVM/TI. Karen and Serguei, when you get the chance, please re-review. Again, thanks for the re-review! Dan

Thumbs up! Coleen On 2/6/2013 12:59 AM, Daniel D. Daugherty wrote: Greetings,

The JPDA stack testing finished with no new regressions on HSX-23.6, HSX-24 or HSX-25. The HSX-24 version of the fix has been pushed. I've updated the HSX-25 version of the fix due to Karen's comments about the MinimalVM configuration in Code Review Round 1. In this latest round for HSX-25, I've made use of the INCLUDEJVMTI define to limit the exposure of new tracing code to configurations that include JVM/TI support. The MinimalVM does not support JVM/TI so none of the new code needs to be included there. While I was at it, I also excluded some other JVM/TI RedefineClasses() support code from the MinimalVM config that I hadn't previously touched. In short, none of the functionality has been changed in this round. Just the way it is built or not built has been changed. Here is the URL for the latest HSX-25 webrev: http://cr.openjdk.java.net/~dcubed/7182152-webrev/2-hsx25/ Thanks, in advance, for more comments, suggestions or questions. Dan

On 2/4/13 2:08 PM, Daniel D. Daugherty wrote: Greetings, I've updated the fix due to comments in Code Review Round 0. Here is a summary of changes made to the various files: src/share/vm/oops/cpCacheOop.cpp (HSX-23.6, HSX-24) src/share/vm/oops/cpCache.cpp (HSX-25) src/share/vm/oops/klassVtable.cpp - removed the new RCTRACENOCR() macro calls at Coleen's request; these files are outside of JVM/TI RedefineClasses proper so the tracing/debug style should not be dictated by JVM/TI RedefineClasses style that is currently in use - did not touch the existing RCTRACE... macros in the file; removing the existing macro calls would be outside the scope of this bug src/share/vm/oops/cpCacheOop.cpp (HSX-23.6, HSX-24) src/share/vm/oops/cpCache.cpp (HSX-25) - copy a comment from ConstantPoolCacheEntry::isinterestingmethodentry() to ConstantPoolCacheEntry::checknooldorobsoleteentries() src/share/vm/oops/klassVtable.cpp - Copied the new comment intended to prevent the "break" from re-materializing again from klassItable::adjustmethodentries() to klassVtable::adjustmethodentries(); yes, I'm paranoid. In the HSX-25 version only: src/share/vm/oops/metadata.hpp - revert changes src/share/vm/oops/cpCacheOop.cpp src/share/vm/oops/klassVtable.cpp - wrap uses of isvalid() in NOTPRODUCT macro as appropriate Here are the URLs for the updated webrevs: http://cr.openjdk.java.net/~dcubed/7182152-webrev/1-hsx23.6/ http://cr.openjdk.java.net/~dcubed/7182152-webrev/1-hsx24/ http://cr.openjdk.java.net/~dcubed/7182152-webrev/1-hsx25/ The JPDA stack testing that I started on Friday is still running on WinXP so I'm blocked for checking the recompile due to these changes. I'll start a recompile on Solaris X86, but that will take some time. Thanks, in advance, for more comments, suggestions or questions. Dan

On 2/1/13 12:55 PM, Daniel D. Daugherty wrote: Greetings, I have a fix for the following JVM/TI bug: 7182152 Instrumentation hot swap test incorrect monitor count http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7182152 https://jbs.oracle.com/bugs/browse/JDK-7182152 The fix for the bug in the product code is one line: src/share/vm/oops/klassVtable.cpp: @@ -992,18 +1020,50 @@ // RCTRACE macro has an embedded ResourceMark RCTRACE(0x00200000, ("itable method update: %s(%s)", newmethod->name()->asCstring(), newmethod->signature()->asCstring())); } - break; + // cannot 'break' here; see for-loop comment above. } ime++; } } } and is applicable to JDK7u10/HSX-23.6 and JDK7u14/HSX-24. Coleen already fixed the bug as part of the Perm Gen Removal (PGR) project in HSX-25. Yes, we found a 1-line bug fix buried in the monster PGR changeset. Many thanks to Coleen for her help in this bug hunt! The rest of the code in the webrevs are: - additional JVM/TI tracing code backported from Coleen's PGR changeset - additional JVM/TI tracing code added by me and forward ported to HSX-25 - a new -XX:TraceRedefineClasses=16384 flag value for finding these elusive old or obsolete methods - exposure of some printing code to the PRODUCT build so that the new tracing is available in a PRODUCT build You might be wondering why the new tracing code is exposed in a PRODUCT build. Well, it appears that more and more PRODUCT bits deployments are using JVM/TI RedefineClasses() and/or RetransformClasses() at run-time to instrument their systems. This bug (7182152) was only intermittently reproducible in the WLS environment in which it occurred so I made the tracing available in a PRODUCT build to assist in the hunt. Raj from the WLS team has also verified that the HSX-23.6 version of fix resolves the issue in his environment. Thanks Raj! Here are the URLs for the three webrevs: http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx23.6/ http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx24/ http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx25/ I have run the following test suites from the JPDA stack on the JDK7u10/HSX-23.6 version of the fix with -XX:TraceRedefineClasses=16384 specified: sdk-jdi sdk-jdiclosed sdk-jli vm-heapdump vm-hprof vm-jdb vm-jdi vm-jdwp vm-jvmti vm-sajdi The tested configs are: {Solaris-X86, WinXP} X {Client VM, Server VM} X {-Xmixed, -Xcomp} X {product, fastdebug} With the 1-liner fix in place, the new tracing code does not find any instances of this failure mode in any of the above test suites. Without the the 1-liner fix in place, the new tracing code finds one instance of this failure mode in the above test suites: test/java/lang/instrument/IsModifiableClassAgent.java There are two new tests that will be pushed to the JDK repos using a different bug ID (not yet filed): test/com/sun/jdi/RedefineAbstractClass.sh test/java/lang/instrument/RedefineSubclassWithTwoInterfaces.sh There will be a separate review request for the new tests. I'm currently running the JPDA stack of tests on the JDK7u14/HSX-24 and JDK8-B75/HSX-25 versions of the fix. That testing will likely take all weekend to complete. Thanks, in advance, for any comments and/or suggestions. Dan

More information about the serviceability-dev mailing list