RFR (S): 8022872: G1: Use correct GC cause for young GC triggered by humongous allocations

Tony Printezis tprintezis at twitter.com
Wed Aug 21 13:32:02 PDT 2013


Glad it was a nice surprise. :-)

BTW, does anyone (Mark?) know whether I can re-instate my OpenJDK 
membership and/or reviewer status? Or do I start from scratch?

Thanks,

Tony

On 8/21/13 12:21 PM, Bengt Rutisson wrote:
>
> Hi Tony,
>
> On 8/21/13 6:30 PM, Tony Printezis wrote:
>> Hi Bengt,
>>
>> (surprise!)
>
> A nice surprise! But a surprise indeed :)
>
>> This is a useful change. Both webrevs look good. I have a slight 
>> preference to the first one. I didn't like the use of 
>> _allocation_failure on the second one here:
>>
>>  984       result = do_collection_pause(word_size, gc_count_before, 
>> &succeeded,
>>  985           GCCause::_allocation_failure);
>>
>> given that the allocation didn't really fail - the eden is just full. 
>> The other use of _allocation_failure (in the 
>> VM_G1CollectorForAllocation closure) really implies that an 
>> allocation did fail and G1 is trying to take evasive action.
>
> Thanks for looking at this! This was exactly the kind of feedback I 
> was looking for. As I already mentioned I'm also fine with the first 
> webrev and with this explanation I'm also in favor of that one. Unless 
> anyone else has opinions I'll go with that one. Just need one more 
> review...
>
> Bengt
>
>>
>> Tony
>>
>>
>>
>>
>> On 8/21/13 7:47 AM, Bengt Rutisson wrote:
>>> Hi All,
>>>
>>> Could I have a couple for reviews for this small change?
>>>
>>> http://cr.openjdk.java.net/~brutisso/8022872/webrev.00/
>>>
>>> This change makes sure that we report "humongous allocation" as the 
>>> GC cause when we trigger a young collection because we are 
>>> attempting a humongous allocation.
>>>
>>> Before the change a log entry for such a young GC looked like this:
>>>
>>> [GC pause (G1 Evacuation Pause) (young) 1364M->1364M(2048M), 
>>> 0.0004200 secs]
>>>
>>> After the change it looks like this:
>>>
>>> [GC pause (G1 Humongous Allocation) (young) 1364M->1364M(2047M), 
>>> 0.0541270 secs]
>>>
>>> While doing this change I realized that the GC cause 
>>> _g1_inc_collection_pause was only used in one more place. The slow 
>>> path for allocations. There is already a cause called 
>>> _allocation_failure, so I suggest that we use that one instead. This 
>>> will eliminate the need for _g1_inc_collection_pause.
>>>
>>> In that case the webrev will look like this:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8022872/webrev_incCause/
>>>
>>> I prefer this cleanup, but I'm fine with just doing the first one if 
>>> we think _g1_inc_collection_pause cause to keep for some reason.
>>>
>>> Thanks,
>>> Bengt
>>
>

-- 
Tony Printezis | Staff Software Engineer | Twitter

@TonyPrintezis
tprintezis at twitter.com



More information about the hotspot-gc-dev mailing list