RFR(S): 8210514: Obsolete SyncVerbose

[David Holmes]

Hi Dan,

On 8/09/2018 8:00 AM, Daniel D. Daugherty wrote:
> On 9/7/18 5:41 PM, coleen.phillimore at oracle.com wrote:
>>
>> I think this looks good. I think as a future RFE we should add logging 
>> judiciously to this code and not preserve the logging that's already 
>> there.  The new logging for this code should look like the log_debug() 
>> etc calls and not with this distracting TEVENT macro.

I have my doubts as to whether UL can be used in such low-level code, 
but that's a different issue to investigate. I'd also be concerned about 
the overhead of UL in such performance critical code, even if logging is 
not enabled. UL is cheap but not free.

> Just to clarify:
> 
> That "distracting TEVENT macro" has an atomic event counter that is
> part of the logging output so that the person doing the debugging
> knows the sequence of TEVENT calls. When dealing with multi-threaded
> output, the system doesn't always issue the output in the same order
> in which the various threads generate it...

The counter is not atomically updated. It's only a heuristic value - if 
we lose a few updates it's no big deal. But the logging itself is only 
partial and I don't understand the rationale. As John points out the 
obscure "v & (v-1) == 0" is true when v is a power of two. So we will 
only print out the information on calls 1, 2, 4, 8, 16 etc. And that 
makes no sense to me from a debugging perspective.

David

> Dan
> 
>>
>> Thanks,
>> Coleen
>>
>> On 9/7/18 5:14 PM, Mikael Vidstedt wrote:
>>> Please review this change which obsoletes the SyncVerbose flag.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210514 
>>> <https://bugs.openjdk.java.net/browse/JDK-8210514>
>>> Webrev: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8210514/webrev.00/open/webrev/ 
>>> <http://cr.openjdk.java.net/~mikael/webrevs/8210514/webrev.00/open/webrev/> 
>>>
>>>
>>> * Background (from bug)
>>>
>>> The experimental SyncVerbose flag can in theory be used to produce 
>>> logging of some synchronization primitives. The flag was convenient 
>>> when the synchronization implementation was implemented and tuned a 
>>> long time ago.
>>>
>>> The SyncVerbose flag no longer serves the purpose it used to, and is 
>>> "Unstable" (the documentation of the flag says so explicitly). It 
>>> should be obsoleted and later removed.
>>>
>>>
>>> Testing: I’m running the normal tier1 testing (still in progress).
>>>
>>> Cheers,
>>> Mikael
>>>
>>
>