RFR: 8149383: Convert TraceBiasedLocking to Unified Logging

[John Rose]

I'm OK as long as we have a tracking bug of at least the same priority as 8149383.
Code duplication is not something we want to have sitting around for a long time.
— John

On Feb 17, 2016, at 9:00 AM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> 
> > If Kim or John is okay with this proposal of enhancing later,
> > should it be necessary, are we all okay with moving forward
> > with this latest version?
> 
> I'm good with moving forward here.
> 
> Dan
> 
> 
> On 2/17/16 8:40 AM, Rachel Protacio wrote:
>> Thanks, David, for reiterating my earlier comment. If it's okay with everyone, I think this falls again into the category of "works now, can be enhanced with a future RFE". Especially since John's suggestion of using LogMessage seems the most likely fix if any (and perhaps an equivalent way to accomplish what Kim's code was getting at?). But LogMessage framework has not yet been committed, so it would make sense to upgrade this implementation, should we deem it necessary, after the framework is added and after the primary task of creating a working "-Xlog:biasedlocking" option has been accomplished.
>> 
>> To reduce the confusion that some have had regarding the fact that the two conditionals differ just in level and not message (for now), I've added the comment
>> // Log at "info" level if not bulk, else "trace" level
>> before each of the four instances. See http://cr.openjdk.java.net/~rprotacio/8149383.02/ <http://cr.openjdk.java.net/%7Erprotacio/8149383.02/>
>> 
>> I have Dan and David as reviewers. If Kim or John is okay with this proposal of enhancing later, should it be necessary, are we all okay with moving forward with this latest version?
>> 
>> Many thanks,
>> Rachel
>> 
>> 
>> On 2/16/2016 8:25 PM, David Holmes wrote:
>>> Folks, 
>>> 
>>> Rachel already covered this in her original email: 
>>> 
>>>> A comment on the code: in order to maintain the existing functionality 
>>>> of the "(TraceBiasedLocking && (Verbose || !is_bulk))" portions of code, 
>>>> it was necessary to create two separate cases in the conversion, one 
>>>> each for the info (regular) and trace (verbose) levels. It has been 
>>>> asked that the functionality be maintained. The logging statements in 
>>>> these chunks do not necessarily have to stay equal to each other in the 
>>>> future, which this would facilitate. 
>>> 
>>> As she says the logging statements for the two cases need not stay the same, so jumping through hoops to try and print the same message under two different conditions seems like a waste of cycles to me. 
>>> 
>>> Cheers, 
>>> David 
>>> 
>>> On 17/02/2016 8:12 AM, John Rose wrote: 
>>>> On Feb 16, 2016, at 1:16 PM, Daniel D. Daugherty 
>>>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com> <mailto:daniel.daugherty at oracle.com> <mailto:daniel.daugherty at oracle.com>> wrote: 
>>>>> 
>>>>>        I really don't like the duplication, but if I remember correctly 
>>>>>        these are macros so we really can't do something like function 
>>>>>        pointers or some other trick here. Ouch! This duplication pattern 
>>>>>        is repeated in several places where the old logging was conditional 
>>>>>        on two different option variables and a biased locking operational 
>>>>>        state variable (is_bulk). 
>>>>> 
>>>>>        I suppose the logging framework doesn't have an API that says 
>>>>>        to log at different levels based on a parameter. Something like: 
>>>> 
>>>> I agree with Dan.  There's got to be a way to factor this code. 
>>>> 
>>>> Perhaps this is a job for LogMessage:  You compose the message, 
>>>> and then conditionally commit it to two places. 
>>>> 
>>>> — John 
>> 
>