RFR (S): 8022872: G1: Use correct GC cause for young GC triggered by humongous allocations
Tony Printezis
tprintezis at twitter.com
Wed Aug 21 20:32:02 UTC 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