[10] RFR (M/L): 8137099: G1 needs to "upgrade" GC within the safepoint if it can't allocate during that safepoint to avoid OoME

[Stefan Johansson]

On 2018-01-10 20:15, Erik Helin wrote:
> On 01/10/2018 05:47 PM, Thomas Schatzl wrote:
>> Hi Erik,
>>
>> On Wed, 2018-01-10 at 16:50 +0100, Erik Helin wrote:
>>> Hi Thomas,
>>>
>>> thanks for taking on this work and cleaning this up!
>>
>> thanks for your review.
>>
>>>
>>> On 01/09/2018 10:52 AM, Thomas Schatzl wrote:
>>>> Hi all,
>>>>
>>>>     Erik Duveblad had some offline comments:
>>>>
>>>> New webrevs:
>>>> http://cr.openjdk.java.net/~tschatzl/8137099/webrev.2_to_3/ (diff)
>>>> http://cr.openjdk.java.net/~tschatzl/8137099/webrev.3/ (full)
>>>
>>> this looks good to me now, pending one small comment: please rename
>>> G1CollectedHeap::no_more_regions_left_for_allocation to
>>> G1CollectedHeap::has_regions_left_for_allocation (and of course
>>> change the method). This way you can use the ! operator in the if
>>> condition, which reads a bit easier.
>>>
>>
>> New webrev:
>> http://cr.openjdk.java.net/~tschatzl/8137099/webrev.3_to_4/ (diff)
>> http://cr.openjdk.java.net/~tschatzl/8137099/webrev.4/ (full)
>
> Looks good, Reviewed.
>
+1
Stefan
> Thanks!
> Erik
>
>>> And thanks for putting this into 11, it is the right decision IMO.
>>>
>>
>> No problem.
>>
>> Thanks,
>>    Thomas
>>
>>> Thanks,
>>> Erik
>>>
>>>> - inverting some conditions in the clauses to read better
>>>>
>>>> - extract out the condition to do the maximally compacting full gc
>>>> added in this change into a separate method.
>>>>
>>>> Erik Duveblad also noted that this change contains some slight
>>>> behavioral change in when a collection is started. I.e. previously
>>>> TLAB
>>>> allocation by itself could not cause a GC. Since this change is
>>>> already
>>>> quite big, he suggested to fix this in a follow-up, and push them
>>>> together.
>>>>
>>>> Thanks,
>>>>     Thomas
>>>>
>>