RFR: 8150103: Convert TraceClassPaths to Unified Logging

[Coleen Phillimore]

On 2/22/16 12:43 AM, David Holmes wrote:
> Hi Max,
>
> On 20/02/2016 6:11 AM, Max Ockner wrote:
>> I have responded to these suggestions.
>> New webrev: http://cr.openjdk.java.net/~mockner/classpath.05/
>>
>>   - ClassLoader::trace_class_path() no longer takes a stream argument.
>> Because the body of trace_class_path is guarded by the classpath tag
>> (formerly TraceClassPaths), the output should always go to
>> LogHandle(classpath)::info_stream().
>>   - SharedPathMiscInfo::trace_class_path() has been removed since it is
>> now redundant. Its only purpose was to call trace_class_path on tty.
>> ClassLoader::trace_class_path is now called in its place.
>>   - print_path() no longer takes a stream argument. It now prints to
>> LogHandle(classpath)::info_stream().
>>   - Accessing LogHandle(classpath)::info_stream() was causing issues
>> with the nonexistant current thread in arguments.cpp, so the single
>> logging statement in arguments.cpp was moved to classLoader.cpp.
>
> That seems like an issue with UL - do we have any clear guidance at 
> what point in VM initialization we can start to use UL?

Yes, Max I think you should file a bug with details about the problem 
that you hit with adding LogStream to arguments.cpp.

Thanks,
Coleen

>
> src/share/vm/classfile/sharedPathsMiscInfo.cpp
>
> I agree with Coleen that the changes in check() should be simplified, 
> and also in add_path(). I think the strings that were printed outside 
> of trace_class_path and print_paths, should just be folded in to what 
> gets passed to trace_class_paths/print_paths, as they appear to be 
> prefixes - they used tty->print not tty->print_cr.
>
> Thanks,
> David
> -----
>
>> Thanks,
>> Max
>>
>> On 2/18/2016 7:28 AM, David Holmes wrote:
>>> Hi Max,
>>>
>>> This conversion is not straight-forward, and not complete.
>>>
>>> On 18/02/2016 8:19 AM, Max Ockner wrote:
>>>> Hello everyone,
>>>> Please review this conversion of TraceClassPaths to UL.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8150103
>>>> Webrev: http://cr.openjdk.java.net/~mockner/classpath.03/
>>>
>>> src/share/vm/classfile/classLoader.cpp
>>>
>>> This call:
>>>
>>>  477     trace_class_path(tty, "[Bootstrap loader class path=",
>>> sys_class_path);
>>>
>>> shouldn't be passing tty but should be the outputstream for the "info"
>>> log.
>>>
>>> ---
>>>
>>> src/share/vm/runtime/arguments.cpp
>>>
>>> Same here:
>>>
>>>   if (!PrintSharedArchiveAndExit) {
>>>     ClassLoader::trace_class_path(tty, "[classpath: ",
>>> _java_class_path->value());
>>>   }
>>>
>>> ---
>>>
>>> src/share/vm/classfile/sharedPathsMiscInfo.hpp
>>>
>>> You didn't modify this file but you need to change the 2-arg
>>> trace_class_path() to not use the tty.
>>>
>>> This conversion seems a bit problematic to me. trace_class_path was
>>> enabled/disabled via TraceClassPaths, but it could write to different
>>> output streams. With UL you would always want it to write to the
>>> single UL related stream - which means the API really needs to change,
>>> and the extraction of the correct log stream can then be internalized.
>>> But there may be some compatibility issues in doing that for the
>>> place(s) where it may not be writing to tty.
>>>
>>> I don't see a simple conversion process here. I think all
>>> trace_class_path calls should be the two-arg forms with no stream arg,
>>> and that version then inlines the current 3-arg version but always
>>> writes to the info log stream. But then you also have print_path which
>>> wraps a call to trace_class_path ... to be honest I don't see what
>>> purpose print_path serves, it just seems to be complicating things.
>>>
>>> David
>>> ----
>>>
>>>
>>>> Summary:  TraceClassPaths flag has been reimplemented using the
>>>> classpath tag with Info level.  TraceClassPaths has been added to the
>>>> logging alias table.
>>>> There is some related output that was never guarded by "if
>>>> (TraceClassPaths)", so it has not been included in this conversion.
>>>> Testing: jtreg runtime tests. Added basic test case at
>>>> test/runtime/logging/ClassPathTest.java
>>>>
>>>> Thanks,
>>>> Max
>>