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

[Bengt Rutisson]

According to this page you are still a Reviewer for the HSX project and 
the JDK projects:

http://openjdk.java.net/census#tonyp

Bengt

On 8/21/13 10:32 PM, Tony Printezis wrote:
> 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
>>>
>>
>