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

Ioi Lam ioi.lam at oracle.com
Thu Mar 10 22:50:56 UTC 2016


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



More information about the hotspot-runtime-dev mailing list