JVM/TI code review request (XS and M) (7182152)
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 INCLUDE_JVMTI. 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 INCLUDE_JVMTI 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 RC_TRACE_NO_CR() 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 RC_TRACE... 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::is_interesting_method_entry()
>>>>> to ConstantPoolCacheEntry::check_no_old_or_obsolete_entries()
>>>>>
>>>>> src/share/vm/oops/klassVtable.cpp
>>>>> - Copied the new comment intended to prevent the "break" from
>>>>> re-materializing again from klassItable::adjust_method_entries()
>>>>> to klassVtable::adjust_method_entries(); 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 is_valid() in NOT_PRODUCT 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/view_bug.do?bug_id=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 @@
>>>>>> // RC_TRACE macro has an embedded ResourceMark
>>>>>> RC_TRACE(0x00200000, ("itable method update: %s(%s)",
>>>>>> new_method->name()->as_C_string(),
>>>>>> new_method->signature()->as_C_string()));
>>>>>> }
>>>>>> - 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-jdi_closed
>>>>>> 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 hotspot-runtime-dev
mailing list