[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR(xs): 8213834: JVMTI ResourceExhausted should not be posted in CompilerThread
From: JC Beyler <jcbeyler () google ! com>
Date: 2018-11-27 5:50:19
Message-ID: CAF9BGBwUTXVzPjW2hHgYjAw9oewxD1Zk2u+Mw4-3AuEmr0QoCg () mail ! gmail ! com
[Download RAW message or body]
Hi Thomas,
Looks good to me too!
Jc
On Mon, Nov 26, 2018 at 3:10 PM David Holmes <david.holmes@oracle.com>
wrote:
> +1 more
>
> Thanks,
> David
>
> On 27/11/2018 3:57 am, serguei.spitsyn@oracle.com wrote:
> > Hi Thomas,
> >
> > +1
> >
> > Thanks,
> > Serguei
> >
> >
> > On 11/26/18 06:53, Daniel D. Daugherty wrote:
> > > On 11/26/18 8:07 AM, Thomas Stüfe wrote:
> > > > Hi guys,
> > > >
> > > > latest webrev:
> > > >
> http://cr.openjdk.java.net/~stuefe/webrevs/8213834-jvmti-reourceexhausted-shall-not-be-posted-from-compiler-thread/webrev.02/webrev/
>
> > > >
> > >
> > > src/hotspot/share/prims/jvmtiExport.cpp
> > > No comments.
> > >
> > > Thumbs up.
> > >
> > > Dan
> > >
> > > >
> > > > Back to can_call_java(), since this seems to be the consensus, with a
> > > > comment added.
> > > >
> > > > As for the Thread::can_send_jvmti_events() property idea, I created a
> > > > RFE to track discussions around this:
> > > > https://bugs.openjdk.java.net/browse/JDK-8214294 but decided to
> > > > postpone this for later. I would like to close that particular issue,
> > > > if possible with a minimal fix which can be easily downported to older
> > > > released.
> > > >
> > > > Thanks, Thomas
> > > >
> > > >
> > > >
> > > >
> > > > On Mon, Nov 19, 2018 at 1:00 PM Thomas Stüfe
> > > > <thomas.stuefe@gmail.com> wrote:
> > > > > Hi all,
> > > > >
> > > > > David and JC already outlined the options we have nicely.
> > > > >
> > > > > I'd like to add that I do not favor the ServiceThread delayed
> > > > > deliverance since a common reaction to ResourceExhausted would to
> > > > > print call stack of the thread running into it or to print a heap
> > > > > histogram, as jvmkill does. For the former only a synchronous event
> > > > > delivery makes sense, for the latter at least it helps analyzing.
> > > > >
> > > > > In cloud foundry, the heap histogram produced by the JVMTI agent can
> > > > > be helpful, and since in the majority of cases do not entail the
> > > > > compiler thread running out of metaspace, I'd rather preserve this
> > > > > ability. So to me, masking this event for this one case is the most
> > > > > pragmatic approach.
> > > > >
> > > > > The other option would be not to change the code but to add, in the
> > > > > JVMTI documentation for ResourceExhausted, the same disclaimer as for
> > > > > ObjectFree, GCStarted etc: "The event handler must not use JNI
> > > > > functions and must not use JVM TI functions except those which
> > > > > specifically allow such use". Then, writers of JVMTI agents like
> > > > > jvmkill would have to update their agents accordingly.
> > > > >
> > > > > FWIW, I think JCs idea of exposing the can_call_java attribute somehow
> > > > > to the outside would also work. But unfortunately not retroactively,
> > > > > for older releases. Whereas a simple internal patch like "mask
> > > > > ResourceExhausted" could be backported easily to older releases.
> > > > >
> > > > > Best Regards, Thomas
> > > > >
> > > > >
> > > > > On Thu, Nov 15, 2018 at 5:32 AM JC Beyler <jcbeyler@google.com>
> wrote:
> > > > > > For what it's worth I wonder if skipping the event is the best; it
> > > > > > is the easiest to ensure non breaking the agent; it does not really
> > > > > > go against what the spec is saying and another thread that CAN call
> > > > > > java will most likely hit the issue and then all will be good in
> > > > > > the world.
> > > > > >
> > > > > > However, also for what it's worth I wonder if deferring is not that
> > > > > > hard to accomplish either. There already is the infrastructure for
> > > > > > this and we should be relatively able to do it. Only question I
> > > > > > would have is can the service thread create a JNIEnv for the event
> > > > > > but I don't think that's an issue, is it?
> > > > > >
> > > > > > It might however conflict with the description of the JNIEnv in the
> > > > > > spec which says the jni_env is "The JNI environment of the event
> > > > > > (current) thread" though it doesn't say current of what or the
> > > > > > event thread of what really.
> > > > > >
> > > > > > However, skipping it also kind of goes against the spec since it
> > > > > > says: "Sent when a VM resource needed by a running application has
> > > > > > been exhausted".Though someone could argue it doesn' t say it is
> > > > > > sent when the resource was first noticed to be exhausted.
> > > > > >
> > > > > > So, if I over-read the spec and look at options, it seems that:
> > > > > > - Sending the event via the compiler thread will risk breaking
> > > > > > things if the agent calls Java -> not really an option
> > > > > > - Using the service thread breaks what David calls the synchronous
> > > > > > assumption of the event
> > > > > > - Skipping the event kind of breaks the sentence that the event is
> > > > > > sent when a VM resource has been exhausted
> > > > > >
> > > > > > So we come back to probably skipping is the best solution since at
> > > > > > least the event remains "synchronous" when you get it.
> > > > > >
> > > > > > (A side note would be perhaps to augment ThreadInfo* with the
> > > > > > "can_call_java" bit and then put in the right spots of the spec
> > > > > > that only threads marked as "can_call_java" can safely call Java).
> > > > > >
> > > > > > Thanks,
> > > > > > Jc
> > >
> >
>
--
Thanks,
Jc
[Attachment #3 (text/html)]
<div dir="ltr">Hi Thomas,<div><br></div><div>Looks good to me \
too!</div><div>Jc</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov \
26, 2018 at 3:10 PM David Holmes <<a \
href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">+1 more<br> <br>
Thanks,<br>
David<br>
<br>
On 27/11/2018 3:57 am, <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> wrote:<br> > Hi Thomas,<br>
> <br>
> +1<br>
> <br>
> Thanks,<br>
> Serguei<br>
> <br>
> <br>
> On 11/26/18 06:53, Daniel D. Daugherty wrote:<br>
>> On 11/26/18 8:07 AM, Thomas Stüfe wrote:<br>
>>> Hi guys,<br>
>>><br>
>>> latest webrev: <br>
>>> <a href="http://cr.openjdk.java.net/~stuefe/webrevs/8213834-jvmti-reourceexhausted-shall-not-be-posted-from-compiler-thread/webrev.02/webrev/" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~stuefe/webrevs/8213834-jv \
mti-reourceexhausted-shall-not-be-posted-from-compiler-thread/webrev.02/webrev/</a> \
<br> >>><br>
>><br>
>> src/hotspot/share/prims/jvmtiExport.cpp<br>
>> No comments.<br>
>><br>
>> Thumbs up.<br>
>><br>
>> Dan<br>
>><br>
>>><br>
>>> Back to can_call_java(), since this seems to be the consensus, with \
a<br> >>> comment added.<br>
>>><br>
>>> As for the Thread::can_send_jvmti_events() property idea, I created \
a<br> >>> RFE to track discussions around this:<br>
>>> <a href="https://bugs.openjdk.java.net/browse/JDK-8214294" \
rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8214294</a> \
but decided to<br> >>> postpone this for later. I would like to close that \
particular issue,<br> >>> if possible with a minimal fix which can be easily \
downported to older<br> >>> released.<br>
>>><br>
>>> Thanks, Thomas<br>
>>><br>
>>><br>
>>><br>
>>><br>
>>> On Mon, Nov 19, 2018 at 1:00 PM Thomas Stüfe <br>
>>> <<a href="mailto:thomas.stuefe@gmail.com" \
target="_blank">thomas.stuefe@gmail.com</a>> wrote:<br> >>>> Hi \
all,<br> >>>><br>
>>>> David and JC already outlined the options we have nicely.<br>
>>>><br>
>>>> I'd like to add that I do not favor the ServiceThread \
delayed<br> >>>> deliverance since a common reaction to ResourceExhausted \
would to<br> >>>> print call stack of the thread running into it or to \
print a heap<br> >>>> histogram, as jvmkill does. For the former only a \
synchronous event<br> >>>> delivery makes sense, for the latter at least \
it helps analyzing.<br> >>>><br>
>>>> In cloud foundry, the heap histogram produced by the JVMTI agent \
can<br> >>>> be helpful, and since in the majority of cases do not entail \
the<br> >>>> compiler thread running out of metaspace, I'd rather \
preserve this<br> >>>> ability. So to me, masking this event for this one \
case is the most<br> >>>> pragmatic approach.<br>
>>>><br>
>>>> The other option would be not to change the code but to add, in \
the<br> >>>> JVMTI documentation for ResourceExhausted, the same \
disclaimer as for<br> >>>> ObjectFree, GCStarted etc: "The event \
handler must not use JNI<br> >>>> functions and must not use JVM TI \
functions except those which<br> >>>> specifically allow such use". \
Then, writers of JVMTI agents like<br> >>>> jvmkill would have to update \
their agents accordingly.<br> >>>><br>
>>>> FWIW, I think JCs idea of exposing the can_call_java attribute \
somehow<br> >>>> to the outside would also work. But unfortunately not \
retroactively,<br> >>>> for older releases. Whereas a simple internal \
patch like "mask<br> >>>> ResourceExhausted" could be \
backported easily to older releases.<br> >>>><br>
>>>> Best Regards, Thomas<br>
>>>><br>
>>>><br>
>>>> On Thu, Nov 15, 2018 at 5:32 AM JC Beyler <<a \
href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a>> \
wrote:<br> >>>>> For what it's worth I wonder if skipping the \
event is the best; it <br> >>>>> is the easiest to ensure non breaking \
the agent; it does not really <br> >>>>> go against what the spec is \
saying and another thread that CAN call <br> >>>>> java will most \
likely hit the issue and then all will be good in <br> >>>>> the \
world.<br> >>>>><br>
>>>>> However, also for what it's worth I wonder if deferring is \
not that <br> >>>>> hard to accomplish either. There already is the \
infrastructure for <br> >>>>> this and we should be relatively able to \
do it. Only question I <br> >>>>> would have is can the service thread \
create a JNIEnv for the event <br> >>>>> but I don't think \
that's an issue, is it?<br> >>>>><br>
>>>>> It might however conflict with the description of the JNIEnv in \
the <br> >>>>> spec which says the jni_env is "The JNI \
environment of the event <br> >>>>> (current) thread" though it \
doesn't say current of what or the <br> >>>>> event thread of what \
really.<br> >>>>><br>
>>>>> However, skipping it also kind of goes against the spec since it \
<br> >>>>> says: "Sent when a VM resource needed by a running \
application has <br> >>>>> been exhausted".Though someone could \
argue it doesn' t say it is <br> >>>>> sent when the resource was \
first noticed to be exhausted.<br> >>>>><br>
>>>>> So, if I over-read the spec and look at options, it seems \
that:<br> >>>>> - Sending the event via the compiler thread will risk \
breaking <br> >>>>> things if the agent calls Java -> not \
really an option<br> >>>>> - Using the service thread breaks what \
David calls the synchronous <br> >>>>> assumption of the event<br>
>>>>> - Skipping the event kind of breaks the sentence that the event \
is <br> >>>>> sent when a VM resource has been exhausted<br>
>>>>><br>
>>>>> So we come back to probably skipping is the best solution since \
at <br> >>>>> least the event remains "synchronous" when you \
get it.<br> >>>>><br>
>>>>> (A side note would be perhaps to augment ThreadInfo* with the \
<br> >>>>> "can_call_java" bit and then put in the right \
spots of the spec <br> >>>>> that only threads marked as \
"can_call_java" can safely call Java).<br> >>>>><br>
>>>>> Thanks,<br>
>>>>> Jc<br>
>><br>
> <br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature" data-smartmail="gmail_signature"><div \
dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic