RFR: 8144874: Reimplement TraceClassResolution with UL

[Max Ockner]

Thanks!
Webrev has been updated to address these changes.
If I need to change some of the multi-line logging to have only one 
decorator, then I will still have to do that.

On 12/17/2015 12:36 PM, Rachel Protacio wrote:
> Hey, Max!
>
> Good overall! Comments file by file:
>
> src/share/vm/classfile/classFileParser.cpp
>
>    You missed the print statement at 5380, and by calling log_info on
>    the one at 5369, added a additional newline on top of the one in the
>    message. I think what you need is a outputStream* logst =
>    LogHandle(classresolve)::info_stream(); and then logst->print() [the
>    first time, with a newline in the message] and logst->print_cr()
>    [the second time, without a newline in the message] to it.  I'm sort
>    of confused about what the original code was getting at, but it
>    seems like it'd be most in line to end up with output like
>         [0.000s][info][classresolve] RESOLVE ...
>         RESOLVE ...
>    As per the discussion David Holmes and are having on my exceptions
>    logging RFR, i.e. we should be decorating only the first line of a
>    multi-line message.
>
So should I leave the second tty line as is?
> src/share/vm/classfile/verifier.cpp
>
>    At 109/111, same as in classFileParser.
Fixed.
>
> src/share/vm/classfile/verifier.hpp
>
>    Maybe follow the syntax of the original comment and instead say
>    "Print output for -Xlog:classresolve=info"? Not a big deal though.
Leaving as is.
>
> src/share/vm/oops/constantPool.cpp
>
>    At 207/211, same as classFileParser.
Fixed.
>
> test/runtime/logging/ClassResolutionTest.java
>
>    You probably need to add "java.lang.ref.WeakReference" and
>    "java.lang.reflect.Method" to the @build line, but there seems to be
>    some debate about that going on in my 8145445 RFR thread right now.
>    But you'll probably need it.
>
The test works without this, and this breaks my code (can't find the 
source for these).
> Also, it might be a good idea to refactor the shouldContain's to a
>    separate function, to avoid code duplication. And I'm not sure that
>    the o.getOutput() line is necessary. I think it does it
>    automatically. But you might want to put a o.shouldHaveExitValue(0);
>    at the end of each ProcessBuilder chunk. Finally, I think you might
>    also add two other ProcessBuilders, one for -Xlog:classresolve=off
>    and one for -XX:-TraceClassResolution. For thoroughness. Then
>    shouldNotContain("[classresolve]") the outputs. You can take a look
>    at
> http://cr.openjdk.java.net/~rprotacio/8145153.01/test/runtime/logging/MonitorInflationTest.java.html
>    to see what I mean.

Good point. I fixed this, and I will probably go back to fix the 
classinit test.
>
> Thanks,
> Rachel
>
> On 12/16/2015 3:44 PM, Max Ockner wrote:
>> Hello all,
>> Please review my code for another Unified Logging conversion.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144874
>> Webrev: http://cr.openjdk.java.net/~mockner/classresolve/
>>
>> Summary: "-XX:+TraceClassResolution" reimplemented using Unified 
>> Logging with classresolve tag and Info level. Support is maintained 
>> for TraceClassResolution using the alias table.
>>
>> Tested with: Selection Resolution tests, jtreg tests. This change 
>> also adds a jtreg test for the implementation of classresolve, and 
>> for the maintained support of TraceClassResolution.
>>
>> Thanks,
>> Max
>