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