8151460: Metaspace counters can have inconsistent values

[Jon Masamitsu]

On 03/23/2016 02:04 AM, Stefan Johansson wrote:
> Hi Jon,
>
> Thanks for taking a look at this.
>
> On 2016-03-22 19:15, Jon Masamitsu wrote:
>> http://cr.openjdk.java.net/~sjohanss/8151460/hotspot.00/test/gc/metaspace/TestPerfCountersAndMemoryPools.java.frames.html 
>>
>>
>> 80 // Adding a second GC due to metadata allocations caused caused by 
>> getting
>> 81 // initial size from the pool. This is needed when running with 
>> -Xcomp.
>> 82 System.gc();
>> 83 assertEQ(getUsed(perfNS), pool.getUsage().getUsed(), "Used out of 
>> sync");
>> 84 assertEQ(getCapacity(perfNS), pool.getUsage().getCommitted(), 
>> "Committed out of sync");
>>
>> The comment (which has an extra "caused" BTW) says that the call to 
>> get the
>> initial size necessitates the System.gc().  Is there reason to 
>> believe that the
>> call to get the used will sometime down the road require another 
>> System.gc()?
>>
> Fixed the extra "caused". This is a fair question and I believe it 
> could, but hopefully it will not cause metaspace to grow and make the 
> capacity numbers get out of sync. An other problem, with no easy 
> solution, would be if within the call to getUsed() something is loaded 
> and added to the used number before returning it. But we haven't seen 
> this and until we do I think the single System.gc() call should be 
> sufficient.

Ok.  Let's not fix what's not broken.

>
>> http://cr.openjdk.java.net/~sjohanss/8151460/hotspot.00/test/gc/metaspace/TestMetaspacePerfCounters.java.frames.html 
>>
>>
>> 87 // Need to ensure that used is up to date and the all unloadable
>> 88 // classes are unloaded before doing this check.
>>
>>
>> I'd suggest "unreachable" or "dead" in place of "unloadable"
>>
> I agree, changed to "unreachable" and "the" to "that".

Thanks.

>
> New webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8151460/hotspot.01/
> Inc: http://cr.openjdk.java.net/~sjohanss/8151460/hotspot.00-01/

Looks good.

Jon

>
> Thanks,
> Stefan
>> // Need to ensure that used is up to date and the all unreachable
>> // classes are unloaded before doing this check.
>>
>> Using "unloadable" feels a little like using an adjective to
>> describe itself.  Kind of like "What classes can be unloaded?
>> The unloadable ones." :-)
>>
>> Jon
>>
>> On 3/22/2016 6:35 AM, Stefan Johansson wrote:
>>> Hi,
>>>
>>> Please review this test-fix to avoid the problems described in:
>>> https://bugs.openjdk.java.net/browse/JDK-8151460
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8151460/hotspot.00/
>>>
>>> Summary:
>>> Two of the metaspace perf-counter tests fails intermittently when 
>>> run with -Xcomp. One reason this has been more frequent lately is a 
>>> change to one of the tests that removed a System.gc() call. This 
>>> call seems to be more important when running with -Xcomp. Having the 
>>> System.gc() call is needed because the PerfCounters are only updated 
>>> after a GC. And if any class loading/unloading happens after the 
>>> call we can't do the assertions we are doing.
>>>
>>> When doing this fix I also realized that one of the tests hadn't 
>>> changed UseCompressedKlassPointers to UseCompressedClassPointers, 
>>> both the command-line and the test-code used the old version so I 
>>> updated this as well.
>>>
>>> Testing:
>>> The updated tests pass in RBT.
>>>
>>> Thanks,
>>> Stefan
>>
>