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

[Coleen Phillimore]

Still looks good to me.
Coleen


On 3/11/16 2:31 PM, Ioi Lam wrote:
> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>