Okay, I created an RFE for this: https://bugs.openjdk.java.net/browse/JDK-8214294
And for now I will prepare a new minimal patch based on our discussions and post it shortly. ..Thomas On Mon, Nov 26, 2018 at 12:28 PM David Holmes <david.hol...@oracle.com> wrote: > > 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 > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>> > >>>> > >>>
