RFR: 8145153: Convert TraceMonitorInflation to Unified Logging

Rachel Protacio rachel.protacio at oracle.com
Fri Dec 11 19:48:01 UTC 2015


Thanks for the reviews, I'll commit it!

As for the alias table, with the current implementation, both a "+" and 
a "-" entry are necessary. The TraceExceptions code in the other thread 
was incomplete in that regard. (I'm working on other fixes to it so 
hadn't yet sent out an updated version.) So yes, Max, if you can make a 
simpler way to map both forms to one entry, that would be great!

I will file an RFE for deprecating both this option and the 
TraceExceptions option.
Thank you!
Rachel

On 12/11/2015 1:10 PM, Coleen Phillimore wrote:
>
>
> On 12/11/15 1:04 PM, Max Ockner wrote:
>> This makes me wonder what the best way is to represent the allowed 
>> flag-setting operations in the alias table.  For example, the 
>> TraceExceptions conversion uses only one line in the alias table and 
>> only supports "-XX:+TraceExceptions". But the current conversion of 
>> TraceMonitorInflation supports both the "+" and "-" forms of the 
>> option.  This is unnecessary at worst, since the flag is False by 
>> default (and the flag doesn't appear to be turned on in any bulk 
>> flag-setting operation, which may necessitate turning off specific 
>> flags to isolate the desired ones.) Either way, It doesn't hurt and 
>> it is a good catch.
>
> Max, I think your idea of processing the aliased options using the 
> programmatic way to turn off and on logging would be a lot nicer after 
> seeing this.  I think you should file an RFE and assign it to yourself 
> to clean this up.
>
> This change looks good and we can use it going forward with the other 
> aliased (for now) options.
>
> Also we need an RFE to deprecate these too, and collect the product 
> logging flags that we're going to deprecate in the RFE.
>
> Thanks,
> Coleen
>>
>> Looks good to me!
>> Max
>>
>> On 12/11/2015 12:41 PM, Rachel Protacio wrote:
>>> Hi David,
>>>
>>> Thanks for the review! Updated webrev: 
>>> http://cr.openjdk.java.net/~rprotacio/8145153.01/
>>>
>>> On 12/10/2015 7:53 PM, David Holmes wrote:
>>>> Hi Rachel,
>>>>
>>>> On 11/12/2015 8:21 AM, Rachel Protacio wrote:
>>>>> Hello!
>>>>>
>>>>> Please review another product flag logging change. This one's pretty
>>>>> small. Converts TraceMonitorInflation to -Xlog:monitorinflation=debug
>>>>
>>>> Conversion looks fine.
>>>>
>>>>> internally, with an alias for the old option. I've again patched in
>>>>> Max's alias table code in the arguments.cpp and .hpp files since my
>>>>> previous changeset with that code is currently on hold.
>>>>
>>>> I still have an unanswered question from the other thread about how 
>>>> the order of options is handled. Primarily whether left-to-right 
>>>> processing prevails?
>>> Yes, it works the same as before the change. E.g.
>>>
>>>    java -XX:+TraceMonitorInflation -XX:-TraceMonitorInflation -version
>>>
>>> produces no logging.
>>>>
>>>> I also strongly believe we should be deprecating these flags at the 
>>>> same time we do the conversion as this is a classic case of 
>>>> deprecation if ever there was one!
>>>>
>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8145153/
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145153
>>>>
>>>> On the test, if you use the testlibrary you must also @build it.
>>>>
>>>> Also java -version is sufficient to see some inflating/deflating 
>>>> happen, and your sample class does nothing to force inflation.
>>> Thanks for pointing that out.
>>> Rachel
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>>
>>>>> Testing: passes jck vm tests, JPRT, and RBT quick and noncolo tests
>>>>>
>>>>> Thank you!
>>>>> Rachel
>>>
>>
>



More information about the hotspot-runtime-dev mailing list