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

[Ioi Lam]

Looks good. Thanks!

- Ioi

On 3/11/16 10:46 AM, Max Ockner wrote:
> Yes I did forget the test. It should be there now.
>
> http://cr.openjdk.java.net/~mockner/8149995.04/test/runtime/logging/ClassLoadPreorderTest.java.html 
>
>
> Thanks,
> Max
>
>
> On 3/11/2016 1:13 PM, Ioi Lam wrote:
>> 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>