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

[Max Ockner]

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