RFR: 8150103: Convert TraceClassPaths to Unified Logging

[Coleen Phillimore]

This looks really good.  It looks like you found a good solution to the 
arguments.cpp problem.

http://cr.openjdk.java.net/~mockner/classpath.05/src/share/vm/classfile/sharedPathsMiscInfo.cpp.udiff.html

I think you can shorten these two changes from:

*!if (_log_is_enabled(Info, classpath)_) {*
*!_ResourceMark rm_;*
*!_outputStream* log = LogHandle(classpath)::info_stream(_);*
*!_log->print("type=%s ", type_name(type)_);*
*+ print_path(type, path);*
}


to:

*
**!__log_info(classpath)_("type=%s ", type_name(type)_);* *
+ print_path(type, path);


*And have print_path use log_info(classpath(() for it's printing.

Then you don't need so many logStreams which we found allocate some 
resource area memory.

Thanks,
Coleen

On 2/19/16 3:11 PM, 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.
>
> 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
>