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

[Coleen Phillimore]

I think this looks really good with two tags.  The alias table handling 
looks fine to me.  I have two minor comments and I don't need to see 
another webrev.

http://cr.openjdk.java.net/~mockner/8149995.03/src/share/vm/memory/metaspaceShared.cpp.udiff.html

*!// Should be class load order as per -_Xlog:classloading+preorder_*


should be

*!// Should be class load order as per -_Xlog:classload+preorder_*


http://cr.openjdk.java.net/~mockner/8149995.03/src/share/vm/runtime/arguments.cpp.udiff.html

Also, can you assign alf instead

AliasedLoggingFlag alf;

if (sscanf(arg, "-%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
alf = catch_logging_aliases(name);


likeif it's used only inside this scope.



if (sscanf(arg, "-%" XSTR(BUFLEN) NAME_RANGE "%c", name, &dummy) == 1) {
AliasedLoggingFlag alf = catch_logging_aliases(name);


Thanks,
Coleen



On 3/10/16 1:22 PM, 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
>>>>
>>>
>>
>