On 26/11/2018 6:58 pm, Thomas Stüfe wrote:
Hi all,
just a general question, would a
virtual bool Thread::can_post_JVMTI_events();
(similar to can_call_java()) not be a clearer solution? It clearly
expresses what we want, and we can, in every Thread child class,
explicitly specify the behavior.
That would be good for when we update the specification to actually
define what threads can post JVM TI events. But I'd prefer for now to
see just a comment and the can_call_java check.
David
..Thomas
On Mon, Nov 26, 2018 at 9:51 AM Thomas Stüfe <thomas.stu...@gmail.com> wrote:
On Mon, Nov 26, 2018 at 4:58 AM serguei.spit...@oracle.com
<serguei.spit...@oracle.com> wrote:
Hi guys,
On 11/25/18 06:17, Daniel D. Daugherty wrote:
On 11/25/18 6:57 AM, David Holmes wrote:
On 25/11/2018 12:41 am, Daniel D. Daugherty wrote:
On 11/22/18 5:24 PM, David Holmes wrote:
Hi Thomas,
On 23/11/2018 2:16 am, Thomas Stüfe wrote:
Hi all,
would such a patch be acceptable:
http://cr.openjdk.java.net/~stuefe/webrevs/8213834-jvmti-reourceexhausted-shall-not-be-posted-from-compiler-thread/webrev.01/webrev/
?
As has been proposed, I am now using
thread->is_hidden_from_external_view() to suppress the event.
I still disagree with this for reason previously outlined. It not
only excludes the CompilerThreads that can't run Java but all
CompilerThreads, and so excludes JVMCI compiler threads that do
everything in Java and so will quite possibly trigger resource
exhaustion.
I thought the CompilerThread is_hidden_from_external_view() is built on
top of can_call_java():
// Hide native compiler threads from external view.
bool is_hidden_from_external_view() const { return
!can_call_java(); }
so is_hidden_from_external_view() does not exclude JVMCI...
Yes sorry my mistake.
There are many confusions around this.
Unfortunately, I make the same mistakes. :(
Of course, the question of whether JVMCI threads should be
participating
in JVM/TI events and other stuff is being debugged and explored at the
current time.
It also excludes the ServiceThread and CodeCacheSweeperThread.
While the latter seems insignificant I think the ServiceThread is
more significant. Those threads are not excluded from posting other
events AFAICS so I don't see why this event should be treated
specially for them.
Hmmm... I don't think the ServiceThread should be posting JVM/TI events
But it already does! It's already posting deferred JVM TI events
explicitly as part of its job, and I see nothing that excludes it
from posting JVM TI events as part of any other Java code it runs.
You're right. I completely forgot about the ServiceThread doing proxy
event posting duty. So I think where we are is switching back to using
can_call_java()
to restrict the posting of ResourceExhausted events. Less limiting than
!is_hidden_from_external_view(), but still limited. I'm reluctantly okay
with this, but we need to hear from Serguei.
I've read all the discussion thread.
Thank you to everyone participating in this discussion - it is very helpful.
Now, I'm convinced that using can_call_java (not
is_hidden_from_external_view)
is right (thanks a lot to David for defending this point!)
I still have a concern though.
If we skip a ResourceExhausted event this way (as in the webrev above)
then can we expect similar event posted on another (executing Java) thread?
I think this usually would happen, but there is no guarantee.
Speaking for metaspace: Before metaspace OOM happens, we already tried
to GC and that failed to recover metaspace so there is no immediate
hope and chances are high subsequent metaspace allocations will fail
too.
If the JIT runs into metaspace OOM, it will cope by bailing out and
leave that particular method uncompiled. I argue that this is not
observable to the user and therefore ResourceExhausted can be
suppressed.
If we, as you fear, do not resend ResourceExhausted, that means that
no-one else tried to allocate metaspace, and that this JIT compilation
was by pure chance the last one to ever want metaspace. Unlikely as it
is, in that case I think it okay suppressing ResourceExhausted and
going on with our lives.
But if Metaspace OOM keep occurring, they most probably will occur
from regular java threads too and that will trigger ResourceExhausted.
Finally, I am not sure whether a pathological case is possible where
only the JIT were to allocate metaspace over and over again, failing
each time, silently swallowing all OOMs and just falling back to
interpreted. But that problem exists today too.
Thanks, Thomas
I have a doubt that we skip it correctly (I need to learn the code more).
Thanks,
Serguei
Dan
David
-----
either. We definitely don't want the ServiceThread running "arbitrary"
event handler code since taking down the ServiceThread could mess up
various things that rely on it running.
I don't have an opinion about the CodeCacheSweeperThread.
dan
Side note, this function apparently should only ever be called from
JavaThreads? To my knowledge we do not guard metaspace against
allocation from non-java-threads, should we then do that?
Not sure how it is arranged but certainly Metaspace::allocate
expects to only be called from a JavaThread as it immediately
checks for
if (HAS_PENDING_EXCEPTION) {
Cheers,
David
-----
I attempted to create a jtreg test for this but this turns out to
be difficult:
Attempting to trigger a metaspace OOM from a compiler thread proved
futile - chances for that to happen are just too low compared to
other
metaspace users. Only way to do this reliably would be to
artificially
allocate a lot of metaspace from the compiler thread - triggered by a
test switch or similar? That would be very ugly though. And then, I
would need to add a jvmti agent to jtreg, or adapt
jtreg/vmTestbase/nsk/jvmti?
Thanks, Thomas
On Thu, Nov 22, 2018 at 4:22 PM Daniel D. Daugherty
<daniel.daughe...@oracle.com> wrote:
On 11/22/18 2:42 AM, David Holmes wrote:
On 22/11/2018 5:36 pm, Thomas Stüfe wrote:
Hi JC,
On Wed, Nov 21, 2018 at 6:03 PM JC Beyler <jcbey...@google.com>
wrote:
Hi Thomas,
<snip>
I actually agree with not using the service thread to be honest,
resource exhaustion seems to be something you'd want to know
sooner
than later.
How about we do both?
- Fix it now so that the compiler thread does not post the
event
because we'd rather not have crashes and that can get backported
- Put out a RFE that would add the information to
ThreadInfo and
work through the process of a CSR/etc. to get it working for
future
versions?
Thanks,
Jc
Makes sense, sure. But both Dan and Serguei voiced opposition
to this
patch. Dan, Serguei, what do you think?
I note neither Dan nor Serguei responded to my response to them :)
I didn't think a response from me was needed. The last thing I
said was:
I would have no problem suppressing the event from a "hidden"
thread
because those threads intentionally try to hide from JVM/TI.
That would
cover the case for the C1 and C2 CompilerThreads, but I don't know
about these newer JVM/CI Compiler threads that actually run Java
code...
I read your combined response to this as not in conflict with
what I said.
The last line of your response:
> So my preferred point solution is still to skip posting
ResourceExhausted
> if the thread can not run Java code.
matches what I said since the C1 and C2 compiler threads are
hidden and
cannot run Java code. We're in agreement.
Dan
Cheers,
David
-----
If we do not find an agreement, I would rather close this bug
as WNF
than push it in without consensus. I would then just fix it
downstream
in our port.
Your proposed RFE would still make sense, but not in jdk12
anymore,
let alone in older releases.
Thanks, Thomas
