RFR: 8142506: Reimplement TraceClassUnloading with Unified Logging.

[David Holmes]

On 13/11/2015 11:42 PM, Max Ockner wrote:
> David,
>
> We chose Trace* and Print* options to convert to Unified Logging because
> those options do most of the logging.  Trace level logging is basically
> the new WizardMode/Verbose. Flags that could once be called either alone
> or with WizardMode can now be logged with different levels. For example,
>
> "-XX:+TraceClassUnloading" will hit code guarded by "if
> (TraceClassUnloading)", but not "if (TraceClassUnloading &&
> WizardMode)". To see logging contained in the WizardMode statement, you
> must specify WizardMode as well.  When This all gets converted to
> Unified Logging,  logging that was once guarded by "if
> (TraceClassUnloading)" will now be accessible with
> "-Xlog:classunload=debug", and logging that was guarded by "if
> (TraceClassUnloading && WizardMode)" will be accessible through
> "-Xlog:classunload=trace".

It is that mapping that I am questioning: why should it not be 
classunload=info as it is the initial level of logging.

> For this particular case, I could also imagine using the Info level for
> the majority of the logging to make it accessible with
> "-Xlog:classunload", and if we wanted to keep the WizardMode output out
> of product, we could either (1) use #ifndef PRODUCT or (2) change to
> develop level for those outputs. However, I don't see a problem with
> allowing the WizardMode output in product builds.  This output is
> currently blocked from product builds by WizardMode, which is a product
> only flag. There are no #ifdefs that we'd have to remove, so I don't
> expect any kind of performance or size issues with this change.

Including non-product code in a product build will affect both size and 
performance, though to what exact extent we can't say a-priori. A 
develop flag in a product build is a constant and so code guarded by eg 
WizardMode can be elided in a product build.

Cheers,
David

> Thanks,
> Max
>
> On 11/13/2015 12:31 AM, David Holmes wrote:
>> Hi Max,
>>
>> I have not looked at the code :) This is more a meta-commentary on the
>> general approach to conversion ...
>>
>> On 12/11/2015 6:23 AM, 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
>>
>> I'm again having trouble reconciling the conversion of non-product
>> logging into product logging. Really not sure it is desirable or useful.
>>
>> More generally although most of our "tracing" flags start with Trace
>> (some start with Print), that should be read simply as "log" or
>> "report" in my opinion. So in the sense that the UL framework defines:
>> info, debug, trace; it is certainly not the case that our "trace"
>> flags should automatically map to a "trace" log level in the first
>> instance. For something like classloading/unloading I would still
>> expect to see basic information at the info level, more detailed info
>> at the debug level, and so forth. So why are we not doing the basic
>> class-unloading logging at the info level?
>>
>> Thanks,
>> David
>>
>>
>>> 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
>>>
>