RFR(S): 8210514: Obsolete SyncVerbose

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Sep 10 11:38:45 UTC 2018



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>> 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