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