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

[Max Ockner]

New webrev @ http://cr.openjdk.java.net/~mockner/ulclassinit02/
Fixed everything that I said I would fix below.

On 11/24/2015 3:40 PM, Rachel Protacio wrote:
> 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".
Thanks. Fixed, though I wonder how much it matters.
> - between lines 194 and 195, I think you need a ResourceMark for the 
> LogHandle stream.
The ResourceMark is defined already, it just isn't part of the diffs 
because it was already there.
> - 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?
Two reasons why I don't think we should do that.
     (1) We don't want to evaluate format strings unless something is 
being logged. I guess if you can find a way to avoid doing this while 
still making the code look nicer then that is OK. I think it would be 
just as bad to write extra lines of code just to ensure that a short 
string isn't duplicated.
     (2) We do not guarantee that these two messages will always be the 
same. A conversation I had with Coleen led me to believe we should keep 
the messages separate.
> - 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?
OK.
>
> 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
>