RFR: 8149383: Convert TraceBiasedLocking to Unified Logging

Rachel Protacio rachel.protacio at oracle.com
Wed Feb 17 17:08:29 UTC 2016


Great. Thanks, Dan and John! Will commit.
Rachel

On 2/17/2016 12:03 PM, John Rose wrote:
> 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 <mailto: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/
>>>
>>> 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>> 
>>>>> 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
>>>
>>
>



More information about the hotspot-runtime-dev mailing list