RFR: 8142976: TraceClassInitialization has been reimplemented with Unified Logging.

[Rachel Protacio]

Hi,

Looks mostly good, just a few comments:

verifier.cpp
- at line 118 (the first "Verification for" line), it should just be 
"print", not "print_cr".
- between lines 194 and 195, I think you need a ResourceMark for the 
LogHandle stream.
- in the sections starting at lines 179 and 608, I appreciate that you 
were minimizing the number of lines, but I think it's a bad idea to have 
duplicates of the logged strings. Do you think you could define the 
strings outside of the logging and pass it to both functions? Another 
possible solution would be to make a function to do that with a 
signature like
     void log_multiple(bool enabled1, outputStream* st1, bool enabled2, 
outputStream* st2, char* msg);
that could do this in a more formalized manner. A function like this 
could be useful for other similar situations as well while we're 
converting flags one by one. Or what are your thoughts on that?
- I think the reordered nesting makes sense.

ClassInitializationTest.java
- nit: can you move the ");"s from the process builder lines onto the 
lines before them?

Thanks,
Rachel

On 11/24/2015 3:09 PM, Max Ockner wrote:
> Hello,
> Please review my new unified logging code:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8142976
> http://cr.openjdk.java.net/~mockner/ulclassinit01/src/share/vm/classfile/verifier.cpp.cdiff.html 
>
> Summary: -XX:+TraceClassInitialization logging has been reimplemented 
> using unified logging under the classinit tag.
>
> In the segment with recursive verification (see verifier.cpp) I 
> reordered the nested if statement to check was_recursively_verified() 
> first. I valued clean code over potentially avoiding a function call 
> to was_recursively_verified.
>
> What do you think?
>
> Tested with:
> jtreg hotspot tests
> new jtreg test for classinit tag
> performance testing with refworkload.
>
> Thanks, Max