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

[Erik Helin]

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.

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