RFR: 8150103: Convert TraceClassPaths to Unified Logging

[Max Ockner]

New webrev: http://cr.openjdk.java.net/~mockner/classpath.06/
I have addressed the changes suggested by Coleen.

Comments below:

On 2/22/2016 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?
>
This definitely seems like an issue that needs to be worked out. As far 
as I know we have not heard anything about when it is safe to use UL.
> 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.

In sharedPathsMiscInfo.cpp, we have this:

void SharedPathsMiscInfo::add_path(const char* path, int type) {
   log_info(classpath)("type=%s ", type_name(type));
   ClassLoader::trace_class_path("add misc shared path ", path);
   write(path, strlen(path) + 1);
   write_jint(jint(type));
}

We would need to evaluate the format string in the log_info call before 
passing into trace_class_path(). Something like this (from 
./closed/share/vm/classfile/classLoaderExt.cpp)

       int name_len = (int)strlen(file_start);
       if (name_len > 0) {
         ResourceMark rm(THREAD);
         char* libname = NEW_RESOURCE_ARRAY(char, dir_len + name_len + 1);
         *libname = 0;
         strncat(libname, dir_name, dir_len);
         strncat(libname, file_start, name_len);
         trace_class_path("library = ", libname);

David, is this what you were thinking? I personally think think this 
looks clunky, but if it's needed to preserve and important property of 
the output, then it shall be done.
>
> 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
>>