RFR(S): 8210514: Obsolete SyncVerbose
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Sep 11 17:34:24 UTC 2018
So I last wrote on this thread:
On 9/8/18 10:15 AM, Daniel D. Daugherty wrote:
> Mikael, I think converting these TEVENT macros to UL might be
> risky from an interactions POV so I'm going to withdraw my request
> that you do that...
I should have been a bit more clear: I'm good with your original
webrev. All of my concerns have been addressed.
Dan
On 9/11/18 12:56 PM, Mikael Vidstedt wrote:
>
> If I dare summarize this it seems like the existing code doesn’t seem
> to have been used recently, doesn’t do what many expect it to, and it
> doesn’t seem like it’s immediately clear that it’s worth shaping it
> up. Anybody against simply removing it?
>
> Cheers,
> Mikael
>
>> On Sep 10, 2018, at 4:38 AM, coleen.phillimore at oracle.com
>> <mailto:coleen.phillimore at oracle.com> wrote:
>>
>>
>>
>> On 9/8/18 11:02 AM, John Rose wrote:
>>> On Sep 8, 2018, at 9:15 AM, Daniel D. Daugherty
>>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>
>>> <mailto:daniel.daugherty at oracle.com>> wrote:
>>>>
>>>>
>>>>
>>>>> and this looks like it supresses output or only prints output if
>>>>> there's a collision in the increment?
>>>>>
>>>>> - if ((v & (v - 1)) == 0) { \
>>>>
>>>> All these years and I never noticed that line of code
>>>> might be wrong. Because 'v' is a local copy of 'ctr'
>>>> that value will never change so that statement will
>>>> always be true.
>>>
>>> That's not wrong; it just isn't doing what you expect.
>>> Check out is_power_of_2 in globalDefinitions.hpp.
>>> It's apparently an exponential backoff trick, to cut
>>> down on output.
>>>
>>> We should replace the puzzler expression (v & (v-1))==0
>>> by an named function call is_power_of_2(v), so that the
>>> next person to look at this code will know what is intended.
>>> (More correctly, perhaps (v == 0) || is_power_of_2(v), but
>>> I think that's not necessary.)
>>
>> I continue to think we should remove the puzzler expression and this
>> tracing code. I'm unappreciative of having to spend time to decode
>> this one line expression in a project with 1,200,000 lines of code.
>> If someone is newly working on this code and wants to log what is
>> happening, they can figure out what logging they need. As for the
>> TEVENT macro:
>>
>> - TEVENT(Inflated enter - park TIMED);
>>
>> What does this even do? Looks like a declaration of type Inflated
>> enter, but that's when the expression breaks down. Oh, maybe it's a
>> string that might mean something.
>>
>> - tty->print_cr("INFO: " #nom " : %d", v); \
>>
>>
>> Why are these just not strings?
>>
>> Again, someone debugging this code (which is old) could find a better
>> set of printing to suit their needs. UL may or may not be too heavy
>> for this code but I believe tty takes out the tty lock also. This
>> tracing is not helpful for reading and understanding the code, which
>> I have tried to do recently. I vote remove.
>>
>> Thanks,
>> Coleen
>>
>>>
>>> — John
>
More information about the hotspot-runtime-dev
mailing list