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

[David Holmes]

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

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