RFR: 8142506: Reimplement TraceClassUnloading with Unified Logging.

[Coleen Phillimore]

Hi,  I looked at this too.

There are two things that need to be fixed, and probably also fixed with 
Ioi's change.  You can start and stop logging through jvmti and through 
other places in serviceability (maybe jcmd ties into 
classLoadingService.cpp).  So we need logic in the places that you 
removed the option to turn on logging for classunload tag:

http://cr.openjdk.java.net/~mockner/classunload/src/share/vm/prims/jvmtiEnv.cpp.udiff.html
http://cr.openjdk.java.net/~mockner/classunload/src/share/vm/services/classLoadingService.cpp.udiff.html

And you should turn on classunload logging in arguments.cpp for 
-Xverbose:class, not just remove it.

Also, the indentation in hotspot is 2 spaces, not 4.

Thanks,
Coleen


On 11/12/15 2:23 PM, harold seigel wrote:
> 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
>>>>
>>>
>>
>