Can this "searchable prefix" be defined in jvmtiTracing with the       rest of the RC_TRACE macros as some descriptive RC_TRACE name?        Doesn't have to be long but this is distracting stuff.   Also, if       you change this searchable prefix, you'd only have to change it       once.
         
      In the cpCache.cpp case, the RC_TRACE_NO_CR(0x00004000, (""))       macro
      calls adds a searchable prefix for each dump line when the dump       code
      is called from JVM/TI RedefineClasses tracing. Other code that       calls
      the cpCache dump code won't get the JVM/TI RedefineClasses tracing
      prefix. The purpose for the prefix is so that all tracing output       for
      a particular tracing level, e.g., 0x00004000, is grep'able       together.
      
      It wouldn't be appropriate to add the "searchable prefix" to the
      jvmtiTraceRedefineClasses.hpp file. The HEX values in JVM/TI
      RedefineClasses tracing are associated with the code being traced.
      
      
          In       jvmtiRedefineClasses.cpp why can't dump_methods just print the       methods without these RC_TRACE macros?      
      Debug output style for JVM/TI RedefineClasses() is       supposed to be done
      using the jvmtiTraceRedefineClasses mechanism. It was inconsistent       for
      dump_methods() to do its output the way it was so I fixed it.
      
         And       some is printed and some is not?     
      No, in this case, every line will have a jvmtiTraceRedefineClasses
      prefix on it when that tracing level is turned on. Example:
      
      RedefineClasses-0x4000: _old_methods --
      RedefineClasses-0x4000: 0 ( -2) public {old} -- virtual void       RedefineSubclassWithTwoInterfacesTarget.<init>()
      RedefineClasses-0x4000: 1 ( 5) public {old} {obsolete} -- virtual       jobject RedefineSubclassWithTwoInterfacesTarget.echo(jobject)
      
      Again, the prefix, "RedefineClasses-0x4000:", exists so that       everything
      at that tracing level is grep'able together.
      
      
         This       is really hard to read.   The indentation came out strange in the       webrev too.
         
    Yes, the original dump_methods() code didn't follow hotspot
      style and I reformatted it since I was changing much of the
      code in the function.
      
      I tend to use "frames" view in webrevs and I don't see any
      issues with indentation.
      
      
      
          It       looks like the call to dump_methods() is covered by one of these       RC_TRACE macros.     
    Yes, I did that intentionally. If I didn't then I would have to       have
      redone all the print code to match jvmtiTraceRedefineClasses       style.
      It was easier to add RC_TRACE_NO_CR() calls where needed and       protect
      the initial call with an RC_TRACE_ENABLED check.
      
      
         Why       isn't that enough?     
      Because all RedefineClasses debug output is supposed to have a
      prefix like this one:
      
      RedefineClasses-0x4000: _old_methods --
      
      
         This       is really distracting because I keep wondering why it's       RC_TRACE_NO_CR (don't file a CR??) rather than reading the code.         Oh, it's no carriage return.   Ugh.     
    You mean like print_cr()? :-)
      
      
         I       still would like dump_methods to always dump all the methods so if       you're debugging this you can temporarily paste this call various       places without trying to figure out which RC_TRACE number to give       it.
         
    Sorry, that's not how debugging in RedefineClasses is supposed       to work.
      dump_methods() was an outlier that needed to be fixed so that it       matched
      the rest of the jvmtiTraceRedefineClasses infrastructure.
      
      Of course, the Serviceability team is welcome to change this       debugging
      code to suite their own style and tastes. I think that would be an       easier
      task if it was all "the same".
      
      Dan
      
      
          
      Coleen
      
                                  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">

(original) (raw)

Adding back the missing aliases and people...


Coleen,

Thanks for the review. Replies embedded below.


On 2/4/13 7:26 AM, Coleen Phillimore wrote:
On 2/1/2013 6: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/

In cpCache.cpp:
RC\_TRACE\_NO\_CR(0x00004000, (""));


Can this "searchable prefix" be defined in jvmtiTracing with the
rest of the RC_TRACE macros as some descriptive RC_TRACE name? 
Doesn't have to be long but this is distracting stuff.   Also, if
you change this searchable prefix, you'd only have to change it
once.




In the cpCache.cpp case, the RC_TRACE_NO_CR(0x00004000, (""))
macro

calls adds a searchable prefix for each dump line when the dump
code

is called from JVM/TI RedefineClasses tracing. Other code that
calls

the cpCache dump code won't get the JVM/TI RedefineClasses tracing

prefix. The purpose for the prefix is so that all tracing output
for

a particular tracing level, e.g., 0x00004000, is grep'able
together.



It wouldn't be appropriate to add the "searchable prefix" to the

jvmtiTraceRedefineClasses.hpp file. The HEX values in JVM/TI

RedefineClasses tracing are associated with the code being traced.






In
jvmtiRedefineClasses.cpp why can't dump_methods just print the
methods without these RC_TRACE macros?



Debug output style for JVM/TI RedefineClasses() is
supposed to be done

using the jvmtiTraceRedefineClasses mechanism. It was inconsistent
for

dump_methods() to do its output the way it was so I fixed it.




And
some is printed and some is not?



No, in this case, every line will have a jvmtiTraceRedefineClasses

prefix on it when that tracing level is turned on. Example:



RedefineClasses-0x4000: _old_methods --

RedefineClasses-0x4000: 0 ( -2) public {old} -- virtual void
RedefineSubclassWithTwoInterfacesTarget.<init>()

RedefineClasses-0x4000: 1 ( 5) public {old} {obsolete} -- virtual
jobject RedefineSubclassWithTwoInterfacesTarget.echo(jobject)



Again, the prefix, "RedefineClasses-0x4000:", exists so that
everything

at that tracing level is grep'able together.






This
is really hard to read.   The indentation came out strange in the
webrev too.




Yes, the original dump_methods() code didn't follow hotspot

style and I reformatted it since I was changing much of the

code in the function.



I tend to use "frames" view in webrevs and I don't see any

issues with indentation.








It
looks like the call to dump_methods() is covered by one of these
RC_TRACE macros.



Yes, I did that intentionally. If I didn't then I would have to
have

redone all the print code to match jvmtiTraceRedefineClasses
style.

It was easier to add RC_TRACE_NO_CR() calls where needed and
protect

the initial call with an RC_TRACE_ENABLED check.






Why
isn't that enough?



Because all RedefineClasses debug output is supposed to have a

prefix like this one:



RedefineClasses-0x4000: _old_methods --






This
is really distracting because I keep wondering why it's
RC_TRACE_NO_CR (don't file a CR??) rather than reading the code.  
Oh, it's no carriage return.   Ugh.



You mean like print_cr()? :-)






I
still would like dump_methods to always dump all the methods so if
you're debugging this you can temporarily paste this call various
places without trying to figure out which RC_TRACE number to give
it.




Sorry, that's not how debugging in RedefineClasses is supposed
to work.

dump_methods() was an outlier that needed to be fixed so that it
matched

the rest of the jvmtiTraceRedefineClasses infrastructure.



Of course, the Serviceability team is welcome to change this
debugging

code to suite their own style and tastes. I think that would be an
easier

task if it was all "the same".



Dan








Coleen






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