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

[Daniel D. Daugherty]

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...

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
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
>>>>>
>>>>
>>>