RFR(S): 8010738: G1: Output for full GCs with +PrintGCDetails should contain perm gen size/meta data change info

[Jon Masamitsu]

On 5/7/2013 8:40 PM, Bengt Rutisson wrote:
>
> Hi John,
>
> This looks good!
>
> One question. Why do we only print the metaspace information for full 
> GCs? I see that this is consistent with the other GCs, but the 
> metaspace information changes over time, so wouldn't it be interesting 
> to print it for every GC?
>
> Also a heads up, as you may know there are a couple of suggested 
> changes to how the capacity and used methods for metaspace should 
> work. This is probably fine with regards to your change since it is 
> similar to the other GCs, but it may mean that the actual output will 
> change.
>
> A couple of minor comments:
>
> The JTreg test does not have an @run tag. What does that mean? How is 
> it executed?
>
> What do you think about renaming _heap_bytes_before_gc to 
> _heap_used_before_gc? Seems more consistent with the other variable 
> names such as _heap_capacity_before_gc.
>
> Similarly, what do you think about renaming the local varialbles 
> *_byte_* to *_used_* in 
> G1CollectorPolicy::print_detailed_heap_transition()?

Sorry, but removing the "bytes" from the name will make me wonder
about the units.  When we have a rule that says "all quantities are in
bytes unless otherwise indicated by the name of the variable", I
will be happy with such a renaming.

Jon

>
> Thanks,
> Bengt
>
>
>
> On 5/7/13 12:23 AM, John Cuthbertson wrote:
>> Hi Everyone,
>>
>> Can I get a couple of volunteers review these fairly straight forward 
>> changes? The webrev can be found at: 
>> http://cr.openjdk.java.net/~johnc/8010738/webrev.0/.
>>
>> Summary:
>> At sustaining engineering's request I've added the change information 
>> about the metadata space to the detailed heap output for full GCs:
>>
>>> 4.896: [Full GC (Allocation Failure) 512M->343M(512M), 4.3640117 secs]
>>>    [Eden: 0.0B(25.0M)->0.0B(97.0M) Survivors: 0.0B->0.0B Heap: 
>>> 512.0M(512.0M)->343.3M(512.0M)], [Metaspace: 1611K->2433K(6448K)]
>>>  [Times: user=5.13 sys=0.15, real=4.36 secs]
>>
>> The information that gets printed mirrors that of the other 
>> collectors. Here's parallel GC:
>>
>>> 33.245: [Full GC (Ergonomics) [PSYoungGen: 133632K->3423K(144640K)] 
>>> [ParOldGen: 348158K->348158K(348160K)] 481790K->351581K(492800K), 
>>> [Metaspace: 1612K->2433K(6448K)], 2.5788089 secs] [Times: user=9.96 
>>> sys=0.00, real=2.58 secs]
>>
>> The information that's printed is the amount of metaspace used before 
>> the GC, the capacity, and the amount of reserved space.
>>
>> The changes should be straight forward when this is backported to 
>> hs24 - namely replacing the call to 
>> MetaspaceAux::print_metaspace_change() to 
>> perm_gen()->print_heap_change().
>>
>> Most of the changes are the result of renaming some of the fields in 
>> G1CollectorPolicy and the local variables in 
>> G1CollectorPolicy::print_detailed_heap_transition().
>>
>> The included regression test just spawns a child java process to run 
>> a very simple system GC test program with G1 and +PrintGCDetails. The 
>> output of the spawned process is checked for "Metaspace".
>>
>> Thanks,
>>
>> JohnC
>
>