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