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

[Nils Eliasson]

Thank you Vladimir and Tobias!

// Nils


On 2019-07-09 12:46, Vladimir Ivanov wrote:
>
>
>> 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
>>>>>>>>
>>>>