RFR: 8141564: Convert TraceItables and PrintVtables to Unified Logging

Coleen Phillimore coleen.phillimore at oracle.com
Wed Dec 2 19:34:32 UTC 2015


Rachel,  There are a lot of changes here.  I have several comments and 
more suggested cleanup that should be done with this work.

In 
http://cr.openjdk.java.net/~rprotacio/8141564/src/share/vm/interpreter/linkResolver.cpp.udiff.html

I did the original refactoring for trace_method_resolution.  What I 
think would be better is to make it an internal static function in 
linkResolver.cpp and not defined in the linkResolver.hpp.  Make the call 
unconditional and pass vtable index in the last parameter as a default 
parameter with default value -1 so the last line in the function is the 
tty->cr() that I had outside the function call (that was pretty icky).  
And put all the ResourceMark and logstream logic inside the 
trace_method_resolution function.  It would never print to tty so 
doesn't need an outputStream parameter for other callers.

So there would be only the one line trace_method_resolution(params) in 
the regular code.

http://cr.openjdk.java.net/~rprotacio/8141564/src/share/vm/logging/logTag.hpp.udiff.html

Can you put these tags near each other?

http://cr.openjdk.java.net/~rprotacio/8141564/src/share/vm/oops/klassVtable.cpp.udiff.html

One of the things that Markus Gronlund's made me think that we should do 
with his cleanup is to stop declaring functions as members of classes if 
they don't need external linkage.  From his RFR(L): 8140485: Class load 
and creation clean up:

"Comment: In my opinion, we should attempt to break the pattern of 
having private functions declared in header files when the private 
function does not need to reach inside "this"."

I think the function log_vtables should follow this and be declared as a 
static internal function in klassVtable.cpp above the function where 
it's used.

It's true that you don't see this very much in the hotspot sources.

The refactoring of log_vtables is good and very much appreciated.

In each of the logging blocks of code, there are about 6 lines that are 
about the same, that print the access flags and whether it's a default 
or overpass method.  (Karen and Lois should give you a TOI about what 
that means!)  Can you add a method to method.hpp that's something like 
print_flags(outputStream* st) and move this printing there, and make 
those blocks call your new function?  That would shorten many of these 
logging blocks of code and would be nice.  You could call this from the 
linkResolver.cpp trace_method_resolution() function too.

http://cr.openjdk.java.net/~rprotacio/8141564/test/runtime/logging/p1/A.java.html

These classes have missing copyrights.

Thanks!
Coleen


On 12/1/15 5:58 PM, Rachel Protacio wrote:
> Hi,
>
> Please review this changeset that converts -XX:+TraceItables and 
> -XX:+PrintVtables flags to -Xlog:itables and -Xlog:vtables.
>
> Open webrev: http://cr.openjdk.java.net/~rprotacio/8141564/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8141564
>
> In case it's not clear, the changes in oops/klassVtable.cpp around 
> 462-498 are a refactoring of the duplicate code from a conditional to 
> a function with a boolean argument.
>
> Thank you,
> Rachel



More information about the hotspot-runtime-dev mailing list