JVM/TI code review request (XS and M) (7182152)
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Feb 5 10:53:29 PST 2013
On 2/5/13 10:03 AM, Daniel D. Daugherty wrote:
> On 2/5/13 10:50 AM, serguei.spitsyn at oracle.com wrote:
>> Both tests look good, I do not see any issues yet.
>> What is missed is a comment explaining what happened
>> when the bug is not fixed and what correct behavior is expected.
>> Maybe it'd make sense to put bad and good output into the comment,
>> not everything, but the most important fragments.
>> It would help a lot to understand what test is doing.
> Serguei,
>
> Thanks for the review!
>
> I put sample failures for both tests into the bug (8007420) a couple
> of days ago. I'd prefer not to clutter up the test with sample outputs
> if that's OK with you.
Thanks!
I'm still suggesting to put a comment explaining what happens in the
buggy case.
Something like:
"All multiple itable elements of the method *echo(String s)* must be
adjusted at a redefinition.
Otherwise, in incorrect implementation case (bug #) some of them may
become old and obsolete.
It should cause the test to fail: crashed, caught by a guaranty,
incorrect results, etc."
I'm sure you can find much better wording for the above.
BTW, I noticed that the test does not check a correctness of the output
so that if
it does not assert or crash then it will probably pass.
Would it make sense to check if the output does not have the sub-string
"version-0"
after the redefinition of the class
*RedefineSubclassWithTwoInterfacesTarget*?
Thanks,
Serguei
>
> Dan
>
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 2/1/13 3:55 PM, Daniel D. Daugherty wrote:
>>> And here is the webrev for the new tests (relative to JDK8-T&L):
>>>
>>> http://cr.openjdk.java.net/~dcubed/8007420-webrev/0-jdk8-tl/
>>>
>>> As always, comments and suggestions are welcome.
>>>
>>> Dan
>>>
>>>
>>> On 2/1/13 4:39 PM, Daniel D. Daugherty wrote:
>>>> > There are two new tests that will be pushed to the JDK repos using
>>>> > a different bug ID (not yet filed):
>>>>
>>>> New bug is now filed:
>>>>
>>>> 8007420 add test for 6805864 to com/sun/jdi, add test for 7182152
>>>> to java/lang/instrument
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007420
>>>> https://jbs.oracle.com/bugs/browse/JDK-8007420
>>>>
>>>> Of course, the tests cannot be pushed until the HSX changes have made
>>>> it into a promoted build and thus available to JDK8-T&L.
>>>>
>>>> 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
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130205/4a192c83/attachment.html
More information about the hotspot-runtime-dev
mailing list