RFR: 8150103: Convert TraceClassPaths to Unified Logging

[David Holmes]

On 19/02/2016 2:36 AM, Max Ockner wrote:
> Hi David,
>
> I agree with what you have said. Allowing trace_class_path to print to
> tty yields a lot of junk output even when classpath logging is not
> turned on.

How does it do that when there is a guard at the start ??

  We now will only want to print to the classpath info stream,
> so given the current logging API does it make sense to hard-code this
> stream into trace_class_path and remove the stream argument? Why should
> we keep the 3 argument version if it is always being used with the same
> stream argument? Do we expect it to someday use additional streams?

As I said below:

 >> 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.

So go ahead with the changes but you may find some additional corner 
cases that are unclear.

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
>