RFR(xs): 8213834: JVMTI ResourceExhausted should not be posted in CompilerThread

Mon Nov 26 10:01:45 UTC 2018

Hi Thomas,

I like this idea.
It will help to control it better and avoid confusions next time.

Thanks,
Serguei

On 11/26/18 00:58, 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.
>
> ..Thomas
>
>
>
>
> On Mon, Nov 26, 2018 at 9:51 AM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>> On Mon, Nov 26, 2018 at 4:58 AM serguei.spitsyn at oracle.com
>> <serguei.spitsyn at 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.daugherty at 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 <jcbeyler at 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
>>>>>>>>>>>