RFR (S): 8004172: Update jstat counter names to reflect metaspace changes

[Erik Helin]

On 02/07/2013 07:49 PM, Mandy Chung wrote:
>
>
> On 2/7/2013 4:36 AM, Erik Helin wrote:
>> Hi Mandy,
>>
>> thanks for the review! See comments inline.
>>
>> On 02/05/2013 09:14 PM, Mandy Chung wrote:
>>
>>> Looks good.  Minor nits:
>>>
>>> gcCapacityOutput1.awk - should the last column in L7 be removed?
>>
>> I don't think so, jmap -gccause outputs the GC cause in the last
>> column, which sometimes can be "No GC".
>>
>> Why do you think it should be removed?
>>
>
> This fix changes from 4 columns "PGCMN    PGCMX     PGC       PC" to 3
> columns "MCMN     MCMX      MC".   Line 7 is an example output that I
> would think the number of columns is expected to match with line 6.  Is
> that right?

I'm sorry, I misread gcCapacityOutput1.awk as gcCauseOutput1.awk (that 
is why my answer makes no sense at all). You are completely right, the 
last column on line 7 should be removed!

New webrev located at:
http://cr.openjdk.java.net/~ehelin/8004172/jdk/webrev.05/

On 02/07/2013 07:49 PM, Mandy Chung wrote:
>> On 02/05/2013 09:14 PM, Mandy Chung wrote:
>>> How are you going to integrate the hotspot and jdk fixes in a
>>> synchronized fashion?  We shall make sure the TL sun/tools/jstat
>>> tests pass at all times.
>>
>> Thanks for catching this. the tests in tl do passes (I was wrong about
>> this). The change will have to be pushed in several steps:
>> 1. Add new metaspace counters (and keep the old ones). This will be
>>    pushed to hotspot-gc.
>> 2. Wait for the change to trickle down to tl.
>> 3. Update the tests and jstat to use the new metaspace counters.
>> 4. Wait for this change to trickle down to hotspot-gc.
>> 5. Remove the old metaspace counters and push this to hotspot-gc.
>>
>> Do you want me to send out an additional webrev for what the hotspot
>> code will look like with both the old and new counters?
>
> Jon may want to review the hotspot code if revised.  I only review the
> jdk change.
>
> Thanks for considering doing a 2-phase hotspot change.  This kind of
> synchronized change is painful.   For this specific change, I'm open to
> temporarily disable these jstat tests for one integration cycle since
> the impact is only in jstat counters and well isolated and that'll ease
> the pain.  i.e. (1) your hotspot change + add jstat tests in
> jdk/test/ProblemList.txt to hotspot-gc and when it's pulled down to TL
> (2) your jdk jstats change + remove jstat tests in
> jdk/test/ProblemList.txt in TL.
>
> Just another option for you to consider.  No issue from me in either
> option.

Thanks, this is will at least make the push a little bit easier! I will 
talk to Alejandro to make sure that jdk/test/ProblemList.txt gets pushed 
all the way to the tl repository.

Thanks,
Erik

>
> Mandy