[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: NotifyFramePop and FramePop events
From: JC Beyler <jcbeyler () google ! com>
Date: 2017-09-07 1:42:11
Message-ID: CAF9BGBzOtFJbSrZ7xH-AaLs5LntMmQrX4ao63LoZVE8HUZVObg () mail ! gmail ! com
[Download RAW message or body]
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@oracle.com <
serguei.spitsyn@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@oracle.com <
> serguei.spitsyn@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@oracle.com <
> > serguei.spitsyn@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@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
> > > >
> > >
> > >
> > >
> >
> >
>
>
[Attachment #3 (text/html)]
<div dir="ltr">Sounds good to me. Thanks for the quick turn around and figuring out \
what you would like to do.<div><br></div><div>Let me know if I can do anything to \
help,</div><div>Jc</div></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Wed, Sep 6, 2017 at 6:01 PM, <a \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <span \
dir="ltr"><<a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_7915551497890051200moz-cite-prefix">Fixing it in the spec is \
wrong.<br> The implementation needs to be fixed.<br>
A straight-forward fix will most likely cause some performance
degradation in interpreter mode.<br>
So, the fix is going to be more tricky.<br>
<br>
If you do not object, I'll assign this bug to my self.<br>
Will see if there is a time to fix it in 10. <br>
<br>
Thanks,<br>
Serguei <br><div><div class="h5">
<br>
<br>
On 9/6/17 16:56, JC Beyler wrote:<br>
</div></div></div><div><div class="h5">
<blockquote type="cite">
<div dir="ltr">Thanks Serguei!
<div><br>
</div>
<div>Just so I know, is your current thought that we should try
to fix the implementation or change the documentation
something like I did?</div>
<div><br>
</div>
<div>Perhaps you don't know yet :)</div>
<div>Jc</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Sep 6, 2017 at 3:07 PM, <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> <span dir="ltr"><<a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote">
<div><span>
<div \
class="m_7915551497890051200m_1698233392147291441moz-cite-prefix">On 9/6/17 10:29, \
JC Beyler wrote:<br> </div>
<blockquote type="cite">
<div dir="ltr">That is only cleared if the state is
enabled for JVMTI_EVENT_FRAME_POP. If you disable it
before, you'd skip that.</div>
</blockquote>
<br>
</span> Agreed.<br>
It is a bug in implementation.<span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>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).</div>
<div><br>
</div>
<div>Am I wrong?</div>
</div>
</blockquote>
<br>
</span> You are right.<br>
I have filed the bug:<br>
<a class="m_7915551497890051200m_1698233392147291441moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8187289" \
target="_blank">https://bugs.openjdk.java.net/<wbr>browse/JDK-8187289</a><br> \
NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is disabled<br>
<br>
<br>
Thanks,<br>
Serguei
<div>
<div class="m_7915551497890051200h5"><br>
<br>
<br>
<blockquote type="cite">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Tue, Sep 5, 2017 at
11:05 PM, <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>
<span dir="ltr"><<a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote">
<div>
<div \
class="m_7915551497890051200m_1698233392147291441m_-5530748387818926672moz-cite-prefix">Hi
JC,<br>
<br>
Thank you for verifying this!
<div>
<div \
class="m_7915551497890051200m_1698233392147291441h5"><br> <br>
<br>
On 9/5/17 11:01, JC Beyler wrote:<br>
</div>
</div>
</div>
<div>
<div \
class="m_7915551497890051200m_1698233392147291441h5"> <blockquote type="cite">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>After looking at the code a bit
more, I don't see a viable way of
really either:</div>
<div> - At notification disabling,
remove all pop event requests</div>
<div> - 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</div>
<div><br>
</div>
<div>Because of that, I'd recommend
just changing the documentation to
highlight this, it might look a
bit like this:</div>
<div><br>
</div>
<div>
<pre>--- \
old/src/share/vm/prims/jvmti.x<wbr>ml 2017-09-05 10:51:05.850810934 \
-0700
+++ new/src/share/vm/prims/jvmti.x<wbr>ml 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></pre>
<pre>Would someone want to review this and tell \
me if I should create a webrev for it? Or tell me hell no ;-) </pre>
</div>
</div>
</blockquote>
<br>
</div>
</div>
I'd say, "hell no" as it looks wrong to me.
;-)<br>
Please, see a comment below.<span><br>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<pre>Thanks!</pre>
<pre>Jc</pre>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Aug
31, 2017 at 3:15 PM, JC Beyler <span \
dir="ltr"><<a href="mailto:jcbeyler@google.com" \
target="_blank">jcbeyler@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>I was asked to raise a
question about NotifyFramePop
and FramePop events and I
thought I would just ask it
here:</div>
<div><br>
</div>
<div>If we imagine we have a
stack frame such as:</div>
<div><br>
</div>
<div>call_depth0</div>
<div> call_depth1</div>
<div> call_depth2</div>
<div> call_depth3</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>Now imagine that we disable
the frame pop event
notification in call_depth2:</div>
<div>
<div>call_depth0</div>
<div> call_depth1</div>
<div> call_depth2</div>
</div>
<div> <a \
href="https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#SetEventNotificationMode" \
target="_blank">SetEventNotificationMode</a><span>(JVMT<wbr>I_DISABLE,
JVMTI_EVENT_FRAME_POP, NULL)</span><br>
</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>However, imagine now that
later down the road, the stack
trace has built itself back up
and we enabled the event:</div>
<div>
<div>
<div>call_depth0</div>
<div> second_call_depth1</div>
<div> second_call_depth2</div>
</div>
<div> <a \
href="https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#SetEventNotificationMode" \
target="_blank">SetEventNotificationMode</a><span>(JVMT<wbr>I_ENABLE, \
JVMTI_EVENT_FRAME_POP, NULL)</span><br>
</div>
</div>
<div><br>
</div>
<div>Then when leaving
second_call_depth1, we
seemingly will issue a frame
pop event.</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</span> No.<br>
The NotifyFramePop request has been already
cleaned at the first call_depth0 pop.<br>
Please, see the following line in the
jvmtiExport.cpp:
JvmtiExport::post_method_exit(<wbr>):<br>
\
ets->clear_frame_pop(cur_frame<wbr>_number);<br> <br>
We have to make another NotifyFramePop
request to get this one.<br>
<br>
Thanks,<br>
Serguei<span><br>
<br>
<br>
<blockquote type="cite">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote">
<div dir="ltr">
<div><br>
</div>
<div>Here is the qualm:</div>
<div> - 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.</div>
<div> - At least the
documentation for
NotifyFramePop (<a \
href="https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#NotifyFramePop" \
target="_blank">https://docs.oracle.com/javas<wbr>e/7/docs/platform/jvmti/jvmti.<wbr>html#NotifyFramePop</a>)
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"</div>
<div><br>
</div>
<div>Therefore the questions
are:</div>
<div><br>
</div>
<div>1) Is this such a corner
case, that we do not wish to
try to fix the documentation
or the code?</div>
<div>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</div>
<div>3) Is this a bug that we'd
like to fix?</div>
<div><br>
</div>
<div>Thanks for your insight,</div>
<div>Jc</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</span></div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic