RFR JDK-8060017 G1: Report heap sizing time

[Eric Caspole]

Hi Thomas,
The new one is here:
  http://cr.openjdk.java.net/~ecaspole/JDK-8060017/07/webrev/

I removed the time recording in a full gc and added the message check 
into the test. The new title is "Expand Heap After Collection" - how is 
that?

Earlier today I added a new enhancement:
  https://bugs.openjdk.java.net/browse/JDK-8139609
for recording the Object Copy expand time.
Regards,
Eric


On 10/14/2015 10:17 AM, Thomas Schatzl wrote:
> Hi Erik,
>
> On Wed, 2015-10-14 at 10:00 -0400, Eric Caspole wrote:
>> Hi Thomas, I have a couple questions below on your comments so I can see
>> how to proceed -
>> Thanks,
>> Eric
>>
>>
>> On 10/14/2015 05:17 AM, Thomas Schatzl wrote:
>>> Hi Eric,
>>>
>>>
>>> some comments:
>>>    - the change only covers gathering timing numbers for expansion at the
>>> end of GC. Expansion may also occur during GC when the space used for
>>> evacuation is not sufficient any more.
>>> Not sure if that is intended, but the CR itself does not indicate that.
>>>
>>> If you think gathering information about time to expand during gc is out
>>> of scope of this CR (that's fine, it was me who suggested that), please
>>> consider adapting the CR and create a new one for tracking the expansion
>>> time during Object Copy.
>>
>> I see what you mean here but I was not intending to add parallel Object
>> Copy expand time here with the way I understood this CR. I'll create a
>> new CR for recording the parallel Object Copy expand time indicated as a
>> sort of sub-timing of Object Copy.
>
> That's fine.
>
>>>    - please add the verification for this log message to the
>>> TestGCLogMessages test.
>>
>> OK.
>
> Thanks.
>
>>>    - maybe a better name for this phase that indicates that it is only
>>> about expansion at the end of the GC could be found?
>>
>> That is a good idea if later on Object Copy expand time is shown too.
>>
>>>    - I do not think it is a good idea to record heap expansion time during
>>> full GC. The PhaseTimes data structure is otherwise only filled in
>>> during young gc, so it's just confusing. (In G1CollectedHeap::
>>> resize_if_necessary_after_full_collection()).
>>
>> Do you mean you don't want to record/display the expand time at all from
>> there or just not record it using the PhaseTimes at that point? It seems
>> like you would still want to show the expand time even for full gc, why not?
>
> While I would like to improve logging for Full GC, at the moment
> G1GCPhaseTimes is only used for measuring young gc pauses, and is
> heavily geared towards it.
>
> So I think it would be better to not measure heap expansion/shrinking
> time for Full GC at this point. Improving logging for Full GC is imo a
> whole different set of changes.
>
> Thanks,
>    Thomas
>
>