RFR (S) JDK-8149023: Event based tracing should cover safepoint begin and end

[David Holmes]

Based on side-channel email discussions I'm happy that there is no 
obvious breakage potential with adding these new events to the safepoint 
code.

Thanks,
David H.
--------

On 15/02/2016 2:09 PM, David Holmes wrote:
> On 12/02/2016 7:41 PM, David Simms wrote:
>>
>> After further discussion with event folks, I updated the
>> "ExecuteVMOperation" event to carry the safepointId (relation).
>>
>> Updated webrev: http://cr.openjdk.java.net/~dsimms/8149023/webrev1/
>>
>> Reply in-line...
>>
>> On 11/02/16 08:09, David Holmes wrote:
>>> Hi David,
>>>
>>> On 10/02/2016 1:45 AM, David Simms wrote:
>>>> Greetings all,
>>>>
>>>> I have small request for review, for adding event based tracing to
>>>> "SafepointSynchronize::begin()" and "end()"
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8149023
>>>> Webrev: http://cr.openjdk.java.net/~dsimms/8149023/webrev0/
>>>
>>> Repeating what I said in email for the public record ...
>>>
>>> I'm always concerned about the overhead of tracing when it is not
>>> being used.
>>>
>>
>> Disabled events have the overhead of an extra stack allocation, followed
>> by a compare, which the branch predictor will take care of.
>> "#INCLUDE_TRACING" may be disabled to compile most of it out.
>>
>>> I'm also concerned that it is not obvious what gets executed when
>>> these events are created, destroyed or committed. We have to be sure
>>> there are no actions that might interfere with the safepoint protocol,
>>> or which might themselves introduce latency into the safepoint process.
>>
>> That concern would be a separate issue for the event frame-work, they
>> are designed to be used any where, any time.
>
> Sorry but it is impossible to write an event system within hotspot that
> can be used "anywhere, anytime" - there are always constraints - as
> recent updates to the bug report show. So when trying to introduce use
> of events in extremely sensitive areas like the safepoint code there is,
> IMHO, a responsibility to do due diligence and verify that the two will
> not conflict in unexpected ways.
>
>>>
>>> A new comment is that the event.commit blocks seems highly repetitive
>>> in many cases - dare I suggest using a macro? :)
>>
>> That would against the Hotspot "Style Guide" and the existing event code.
>
> The hotspot style guide says:
>
> "You can almost always use an inline function or class instead of a
> macro. Use a macro only when you really need it."
>
> So feel free to introduce an inline function of class if you prefer. :)
>
> I'm afraid I don't find the existing verbosity of event code as
> continuing justification for repetitive code fragments.
>
> Cheers,
> David
>
>> /Mr. Simms
>>