RFR(S): 8210514: Obsolete SyncVerbose
Mikael Vidstedt
mikael.vidstedt at oracle.com
Tue Sep 11 16:56:15 UTC 2018
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 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>> 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