RFR: 8142506: Reimplement TraceClassUnloading with Unified Logging.

[harold seigel]

Hi Max,

Thanks for fixing this.  Just a couple of questions.

    1. In Arguments::loop_logging_aliases, why not use strcpy() instead
    of using your own loop to do the copy.

    2. If "-verbose:class" is specified, will ClassUnloading still be
    traced?  It looks like that code got removed.

    3. In classLoadingService.cpp, is reset_trace_class_unloading() now
    a no-op?  It doesn't look like it does anything useful.

    4. In ClassUnloadTest.java, I think the copyright should start at
    the first line in the file.

Thanks, Harold

On 11/12/2015 2:05 PM, Max Ockner wrote:
> Thanks, Rachel.
>
> I have addressed these concerns, rebuilt, and retested with jtreg 
> runtime tests.
> New webrev:  http://cr.openjdk.java.net/~mockner/classunload/
>
> On 11/11/2015 3:40 PM, Rachel Protacio wrote:
>> Hi,
>>
>> Looks good!
>>
>> I have a few mostly aesthetic comments:
>> 1. Can you put the conditional part of the if statements in 
>> parentheses for clarity throughout? e.g. 
>> src/share/vm/code/nmethod.cpp:1322
>> 2. src/share/vm/logging/logTag.hpp - It looks like there was an 
>> incomplete merge
>> 3. In the test, the comment parts around line 26 are oddly indented. 
>> And if you can add a line for "@bug 8142506".
>>
>> Thanks,
>> Rachel
>>
>> On 11/11/2015 3:23 PM, Max Ockner wrote:
>>> Hello,
>>> Please review my change.
>>>
>>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8142506
>>> Webrev:  http://cr.openjdk.java.net/~mockner/classunload/
>>>
>>> Testing:
>>> Hotspot jtreg tests with and without "-Xlog:classunload=trace"
>>> runthese tests with "-Xlog:classunload=trace"
>>> new jtreg test for classunload output.
>>>
>>> Summary:
>>>
>>> There are two parts to this fix.
>>>
>>> (1) Existing logic for TraceClassUnloading has been replaced with 
>>> commands from Unified Logging.
>>>
>>> (2) Logging Alias Table. "-XX:+TraceClassUnloading" is now mapped to 
>>> "-Xlog:classunload=debug" in the new logging alias table in 
>>> arguments.cpp. Arguments are checked in the beginning of 
>>> parse_each_vm_init_arg and if an argument (such as 
>>> -XX:+TraceClassUnloading") is found in the alias table, the 
>>> optionString is substituted for the value found in the logging alias 
>>> table.
>>>
>>> I had to make a few decisions along that might raise questions, so I 
>>> will knock a few of them out now.
>>>
>>> Why didn't I use the existing alias table?
>>> - Existing alias table only stores the flag name 
>>> (TraceClassUnloading) and is meant for substituting one "-XX:" 
>>> argument for another. This does not solve our problem.
>>>
>>> Why didn't I process the logging aliases later in 
>>> parse_each_vm_init_arg, next to the rest of the "-XX:" argument 
>>> processing?
>>> - The main processing for "-XX:" options happens after processing of 
>>> "-Xlog" options. If we want the unified logging alias to be handled, 
>>> it needs to be substituted in before "-Xlog:" is handled.
>>>
>>> Why does "-XX:+TraceClassUnloading" get aliased to 
>>> "-Xlog:classunload=debug" and not "-Xlog:classunload=trace"?
>>> - logging statements that contained the develop-only flag 
>>> "WizardMode" were converted to "-Xlog:classunload=trace". The 
>>> logging statements that could have been previously been triggered in 
>>> Product mode are all now accessed by "-Xlog:classunload=debug". (We 
>>> are updating logging, but we are not updating the functionality of 
>>> the old logging options. Old options such as TraceClassUnloading are 
>>> being aliased so they may continue to work in *exactly* the way they 
>>> did before.)
>>>
>>> Thanks,
>>> Max
>>>
>>
>