8151460: Metaspace counters can have inconsistent values

[Stefan Johansson]

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.

> 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".

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

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
>