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

[Vladimir Ivanov]

> So here is a version using methodhandles instead:
> 
> http://cr.openjdk.java.net/~neliasso/8226536/webrev.04/

You could do MethodHandle lookup just once (e.g., as part of 
initialization and put MethodHandle instance into a field), but current 
version looks good as well.

Best regards,
Vladimir Ivanov

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