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