NotifyFramePop and FramePop events

JC Beyler jcbeyler at google.com
Thu Sep 7 01:42:11 UTC 2017


Sounds good to me. Thanks for the quick turn around and figuring out what
you would like to do.

Let me know if I can do anything to help,
Jc

On Wed, Sep 6, 2017 at 6:01 PM, serguei.spitsyn at oracle.com <
serguei.spitsyn at oracle.com> wrote:

> Fixing it in the spec is wrong.
> The implementation needs to be fixed.
> A straight-forward fix will most likely cause some performance degradation
> in interpreter mode.
> So, the fix is going to be more tricky.
>
> If you do not object, I'll assign this bug to my self.
> Will see if there is a time to fix it in 10.
>
> Thanks,
> Serguei
>
>
> On 9/6/17 16:56, JC Beyler wrote:
>
> Thanks Serguei!
>
> Just so I know, is your current thought that we should try to fix the
> implementation or change the documentation something like I did?
>
> Perhaps you don't know yet :)
> Jc
>
> On Wed, Sep 6, 2017 at 3:07 PM, serguei.spitsyn at oracle.com <
> serguei.spitsyn at oracle.com> wrote:
>
>> On 9/6/17 10:29, JC Beyler wrote:
>>
>> That is only cleared if the state is enabled for JVMTI_EVENT_FRAME_POP.
>> If you disable it before, you'd skip that.
>>
>>
>> Agreed.
>> It is a bug in implementation.
>>
>>
>> When I go look at the jvmtiEventController and peruse, it seems that if
>> you can have the case I am talking about if all the threads turn off the
>> JVMTI_EVENT_FRAME_POP, then the state would be off as well and then we
>> would not clear (or even check for that matter).
>>
>> Am I wrong?
>>
>>
>> You are right.
>> I have filed the bug:
>>   https://bugs.openjdk.java.net/browse/JDK-8187289
>>     NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is
>> disabled
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>>
>> On Tue, Sep 5, 2017 at 11:05 PM, serguei.spitsyn at oracle.com <
>> serguei.spitsyn at oracle.com> wrote:
>>
>>> Hi JC,
>>>
>>> Thank you for verifying this!
>>>
>>>
>>>
>>> On 9/5/17 11:01, JC Beyler wrote:
>>>
>>> Hi all,
>>>
>>> After looking at the code a bit more, I don't see a viable way of really
>>> either:
>>>    - At notification disabling, remove all pop event requests
>>>    - Enforcing that the current frame at depth is singled out, if you
>>> disable and then pop that frame, the event is destroyed with the frame so
>>> that a subsequent pop at that depth no longer fires
>>>
>>> Because of that, I'd recommend just changing the documentation to
>>> highlight this, it might look a bit like this:
>>>
>>> --- old/src/share/vm/prims/jvmti.xml	2017-09-05 10:51:05.850810934 -0700
>>> +++ new/src/share/vm/prims/jvmti.xml	2017-09-05 10:51:05.710811367 -0700
>>> @@ -2907,7 +2907,7 @@
>>>      <function id="NotifyFramePop" num="20">
>>>        <synopsis>Notify Frame Pop</synopsis>
>>>        <description>
>>> -	When the frame that is currently at <paramlink id="depth"></paramlink>
>>> +	When a frame currently at <paramlink id="depth"></paramlink>
>>>          is popped from the stack, generate a
>>>  	<eventlink id="FramePop"></eventlink> event.  See the
>>>  	<eventlink id="FramePop"></eventlink> event for details.
>>> @@ -2916,6 +2916,12 @@
>>>          <p/>
>>>          The specified thread must either be the current thread
>>>          or the thread must be suspended.
>>> +        <p/>
>>> +        Note: if the notification event is disabled and a frame at
>>> +        <paramlink id="depth"></paramlink> is popped, no event is generated.
>>> +        After re-enabling the notification event, the registered
>>> +        <paramlink id="depth"></paramlink> is still active and will provoke an
>>> +        event when a frame at <paramlink id="depth"></paramlink> is popped.
>>>        </description>
>>>        <origin>jvmdi</origin>
>>>        <capabilities>
>>>
>>> Would someone want to review this and tell me if I should create a webrev for it? Or tell me hell no ;-)
>>>
>>>
>>> I'd say, "hell no" as it looks wrong to me. ;-)
>>> Please, see a comment below.
>>>
>>>
>>> Thanks!
>>>
>>> Jc
>>>
>>>
>>> On Thu, Aug 31, 2017 at 3:15 PM, JC Beyler <jcbeyler at google.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> I was asked to raise a question about NotifyFramePop and FramePop
>>>> events and I thought I would just ask it here:
>>>>
>>>> If we imagine we have a stack frame such as:
>>>>
>>>> call_depth0
>>>>   call_depth1
>>>>     call_depth2
>>>>        call_depth3
>>>>
>>>> And at this third depth, we request a frame pop when leaving depth1 via
>>>> the NotifyFramePop call. We would of course assume that when leaving
>>>> call_depth1 we get a FramePop event.
>>>>
>>>> Now imagine that we disable the frame pop event notification in
>>>> call_depth2:
>>>> call_depth0
>>>>   call_depth1
>>>>     call_depth2
>>>>        SetEventNotificationMode
>>>> <https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#SetEventNotificationMode>
>>>> (JVMTI_DISABLE, JVMTI_EVENT_FRAME_POP, NULL)
>>>>
>>>> If the stack now pops back to call_depth0, the frame pop system is not
>>>> checked, the FramePop for call_depth1 is not issued either.
>>>>
>>>> However, imagine now that later down the road, the stack trace has
>>>> built itself back up and we enabled the event:
>>>> call_depth0
>>>>   second_call_depth1
>>>>     second_call_depth2
>>>>        SetEventNotificationMode
>>>> <https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#SetEventNotificationMode>
>>>> (JVMTI_ENABLE, JVMTI_EVENT_FRAME_POP, NULL)
>>>>
>>>> Then when leaving second_call_depth1, we seemingly will issue a frame
>>>> pop event.
>>>>
>>>
>>> No.
>>> The NotifyFramePop request has been already cleaned at the first
>>> call_depth0 pop.
>>> Please, see the following line in the jvmtiExport.cpp:
>>> JvmtiExport::post_method_exit():
>>>           ets->clear_frame_pop(cur_frame_number);
>>>
>>> We have to make another NotifyFramePop request to get this one.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>
>>>> Here is the qualm:
>>>>   - It seems counter intuitive and the documentation does not
>>>> specify/warn about this; it seems that if you disable the events you should
>>>> perhaps clear up the pop requests.
>>>>   - At least the documentation for NotifyFramePop (
>>>> https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.
>>>> html#NotifyFramePop) should be changed since it says: "When the frame
>>>> that is currently at depth is popped from the stack" to something like
>>>> "When the first frame at the depth is popped from the stack and the event
>>>> notification is enabled"
>>>>
>>>> Therefore the questions are:
>>>>
>>>> 1) Is this such a corner case, that we do not wish to try to fix the
>>>> documentation or the code?
>>>> 2) Is this a corner case we do not wish to handle, therefore put a fix
>>>> in the documentation to at least warn users of this
>>>> 3) Is this a bug that we'd like to fix?
>>>>
>>>> Thanks for your insight,
>>>> Jc
>>>>
>>>
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20170906/bc4f2e49/attachment-0001.html>


More information about the serviceability-dev mailing list