[13] RFR(S): 8226536: Catch OOM from deopt that fails rematerializing objects

[Nils Eliasson]

ok,

I first tried the @DontInline annotation since that captures the 
intention clearly. That failed spectacularly, the GarbageUtil was used 
by an immense amounts of tests that all would have required the @module tag.

So here is a version using methodhandles instead:

http://cr.openjdk.java.net/~neliasso/8226536/webrev.04/

// Nils

On 2019-07-05 13:18, Vladimir Ivanov wrote:
>
>> Sounds like properly annotating this will clobber a lot of tests (~40?).
>> Not sure it's worth the hassle when this RFE is only about silencing 
>> the tests (which it does as the method is too large to get inlined), 
>> as opposed to fixing the underlying issue.
>> The worst that could happen is that the tests start moaning again 
>> with new inlining heuristics, which is okay IMO.
>
> I'd prefer to introduce a reliable fix and not bother with it again.
>
> If the concern is the amount of changes needed, calling eatMemoryImpl 
> using a non-constant MethodHandle (cached in a non-static non-final 
> field) would block inlining as well.
>
> Best regards,
> Vladimir Ivanov
>
>> On 2019-07-05 12:20, Vladimir Ivanov wrote:
>>>> Doesn't that only work on classes loaded by the Bootstrap classloader?
>>>
>>> Yes, that would require putting GarbageUtils on boot class path or 
>>> running affected tests with main/bootclasspath/othervm.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>>>
>>>> // Nils
>>>>
>>>> On 2019-07-05 12:00, Vladimir Ivanov wrote:
>>>>> test/hotspot/jtreg/vmTestbase/nsk/share/gc/gp/GarbageUtils.java:
>>>>>
>>>>> +          * It is Important that the impl is not inlined.
>>>>>
>>>>> What do you think about putting @DontInline on eatMemoryImpl to 
>>>>> guarantee that invariant?
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>> On 04/07/2019 16:27, Nils Eliasson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This is a patch of GarbageUtils.java used by a bunch of GC tests.
>>>>>>
>>>>>> This issue was diagnosed by Stefan Karlsson and passed to the 
>>>>>> compiler team.
>>>>>>
>>>>>> The core problem is that the test relies on successfully catching 
>>>>>> OOM errors. But the OOM causes a deopt, and the deopt must 
>>>>>> rematerialize some objects before passing the execution on to the 
>>>>>> interpreter to run the catch-block. If the rematerialization 
>>>>>> can't allocate, it will pass the OOM on, without having run the 
>>>>>> catch-block.
>>>>>>
>>>>>> With this patch I fix the tests by wrapping the faulting code 
>>>>>> with an extra layer of try-catch-OOM. It relies on not inlining 
>>>>>> the inner method, because then it would be part of the compiled 
>>>>>> frame being deoptimized.
>>>>>>
>>>>>> In general this is a problem for any kind of deopt when we are 
>>>>>> out of heap. Another bug will be opened to keep tracking this issue.
>>>>>>
>>>>>> Webrev:http://cr.openjdk.java.net/~neliasso/8226536/webrev.02/ 
>>>>>> <http://cr.openjdk.java.net/~neliasso/8226536/webrev.02/>
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8226536
>>>>>>
>>>>>> // Nils
>>>>>>
>>