RFR: 8149995: TraceClassLoadingPreorder has been converted to Unified Logging.

[Ioi Lam]

Hi Max,

The code change looks good. Did you forget to include the test in the 
webrev?

Thanks
- Ioi

On 3/11/16 8:14 AM, Max Ockner wrote:
> Ioi,
>
> Thanks for your comments. I have made the change you suggested, and it 
> is reflected in the test.
> webrev: http://cr.openjdk.java.net/~mockner/8149995.04/
>
> Thanks,
> Max
>
> On 3/10/2016 5:50 PM, Ioi Lam wrote:
>> Hi Max,
>>
>> The changes look good. I just have one comment:
>>
>> $ java -Xlog:classload | less
>> [0.108s][info][classload] java.lang.Object source: 
>> /jdk/bld/jdk9_rt/images/jdk/lib/modules/bootmodules.jimage
>> ...
>>
>> For consistency, I think the preorder output should us the same 
>> style. I.e., remove "loading" and replace "from" to "source:"
>>
>> 5686   if (!is_internal()) {
>> 5687     if (log_is_enabled(Debug, classload, preorder)){
>> 5688       ResourceMark rm(THREAD);
>> 5689       outputStream* log = LogHandle(classload, 
>> preorder)::debug_stream();
>> 5690       log->print("loading %s", 
>> _class_name->as_klass_external_name());
>> 5691       if (stream->source() != NULL) {
>> 5692         log->print(" from %s", stream->source());
>> 5693       }
>> 5694       log->cr();
>> 5695     }
>> 5696
>>
>> Thanks
>> - Ioi
>>
>>
>> On 3/10/16 10:22 AM, Max Ockner wrote:
>>> Hello,
>>>
>>> I have responded to these comments.
>>>
>>> webrev: http://cr.openjdk.java.net/~mockner/8149995.03/
>>>  - classloadingpreorder replaced by classload+preorder
>>>  - In order to support aliasing to a log configuration with multiple 
>>> tags, the logging alias table is expanded to fit the output of the 
>>> LOG_TAGS(tag) macro.
>>> - comment in metaspaceShared is changed to classload+preorder.
>>>
>>> Thanks,
>>> Max
>>>
>>> On 3/9/2016 6:58 PM, Coleen Phillimore wrote:
>>>>
>>>> http://cr.openjdk.java.net/~mockner/8149995.02/src/share/vm/classfile/classFileParser.cpp.udiff.html 
>>>>
>>>>
>>>> We talked about this but I've changed my opinion.   I think looking 
>>>> at this in the code, it might make more sense to make this 
>>>> log(classload, preorder) since anyone knowing about this option 
>>>> will know to specify both tags.  I think it's used for CDS.  Will 
>>>> this work for the aliasing table? If not, I think the tag is better 
>>>> classloadpreorder
>>>>
>>>> http://cr.openjdk.java.net/~mockner/8149995.02/src/share/vm/memory/metaspaceShared.cpp.udiff.html 
>>>>
>>>>
>>>> The comment is wrong, it says classloaderpreorder, rather than 
>>>> classloadingpreorder.
>>>>
>>>> thanks,
>>>> Coleen
>>>>
>>>> On 3/9/16 12:10 PM, Max Ockner wrote:
>>>>> Hello again!
>>>>>
>>>>> webrev:  http://cr.openjdk.java.net/~mockner/8149995.02/
>>>>>  - Added "ResourceMark rm(THREAD);" in classFileParser.cpp
>>>>>  - Deleted TraceClassLoadingPreorder flag from globals.hpp
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>>
>>>>> On 3/8/2016 2:55 PM, Rachel Protacio wrote:
>>>>>> Hey, Max,
>>>>>>
>>>>>> Looks good. Just a few comments:
>>>>>>
>>>>>> In src/share/vm/classfile/classFileParser.cpp should this have a 
>>>>>> ResourceMark rm(THREAD) at 5688? I realize there is probably one 
>>>>>> farther up in the scope, but in case someone were to delete it 
>>>>>> later, it might make sense to have a more local one? Not sure 
>>>>>> what the standard is on this.
>>>>>>
>>>>>> Also I think you forgot to delete the option from 
>>>>>> runtime/globals.hpp.
>>>>>>
>>>>>> Thanks,
>>>>>> Rachel
>>>>>>
>>>>>>
>>>>>> On 3/8/2016 8:33 AM, Max Ockner wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please review this small Unified Logging change. 
>>>>>>> TraceClassLoadingPreorder has been converted to UL at the Debug 
>>>>>>> level. Debug level was chosen since there are 342 lines of 
>>>>>>> output for java -version.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8149995
>>>>>>> webrev: http://cr.openjdk.java.net/~mockner/8149995/
>>>>>>>
>>>>>>> Tested with jtreg runtime tests.
>>>>>>> No existing tests reference TraceClassLoadingPreorder from 
>>>>>>> hotspot/test or jdk/test.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Max
>>>>>>
>>>>>
>>>>
>>>
>>
>