RFR: 8264008: Incorrect metaspace statistics after JEP 387 when UseCompressedClassPointers is off

[Thomas Stuefe]

On Tue, 23 Mar 2021 03:50:36 GMT, Jie Fu <jiefu at openjdk.org> wrote:

> Hi all,
> 
> Metaspace statistics is incorrect after JEP 387 when UseCompressedClassPointers is off.
> 
> For example, here is the incorrect metaspace statistics before the fix:
> Event:jdk.MetaspaceSummary {
>   startTime = 10:35:24.762
>   gcId = 3
>   when = "Before GC"
>   gcThreshold = 21.0 MB
>   metaspace = {
>     committed = 10.3 MB
>     used = 10.2 MB
>     reserved = 16.0 MB
>   }
>   dataSpace = {
>     committed = 10.3 MB
>     used = 10.2 MB
>     reserved = 16.0 MB
>   }
>   classSpace = {
>     committed = 10.3 MB
>     used = 10.2 MB
>     reserved = 16.0 MB
>   }
> }
> 
> This bug can be reproduced by running the following tests with `-XX:-UseCompressedClassPointers`, which would pass before JEP 387.
> jdk/jfr/event/gc/heapsummary/TestHeapSummaryEventDefNewSerial.java
> jdk/jfr/event/gc/heapsummary/TestHeapSummaryEventG1.java
> jdk/jfr/event/gc/heapsummary/TestHeapSummaryEventPSParOld.java
> 
> The failing reason is that `Metaspace::is_class_space_allocation(mdtype)` [1] will always return false when `UseCompressedClassPointers` is off.
> So `RunningCounters::committed_words_nonclass()` will be returned even called with `Metaspace::is_class_space_allocation(Metaspace::ClassType)`, which is unreasonable.
> And `MetaspaceUtils::reserved_words(Metaspace::MetadataType mdtype)`/`MetaspaceUtils::used_words(Metaspace::MetadataType mdtype)` also suffer from the same bug.
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/metaspace.cpp#L90
> 
> 
> Metaspace statistics after the fix:
> Event:jdk.MetaspaceSummary {
>   startTime = 10:44:38.230
>   gcId = 1
>   when = "After GC"
>   gcThreshold = 21.0 MB
>   metaspace = {
>     committed = 10.3 MB
>     used = 10.2 MB
>     reserved = 16.0 MB
>   }
>   dataSpace = {
>     committed = 10.3 MB
>     used = 10.2 MB
>     reserved = 16.0 MB
>   }
>   classSpace = {
>     committed = 0 bytes
>     used = 0 bytes
>     reserved = 0 bytes
>   }
> }

Yes, this makes sense. Thanks for fixing this.

The problem is that `is_class_space_allocation` is ambiguous. The way it is implemented it answers the question "should this be allocated from class space". But in this statistics, it is used as a simple "is this a class space type".

What concerns me is that we did not find this earlier. Can you please add a test run for at least one of the failing tests to also execute with compressed oops off?

Even better would be an own regression test, e.g. as part of the gtests. Can be very simple, if UseCompressedClassPointers is off, MetaspaceUtils::committed_words should return 0 for class type. Then we can add another gtest run to the gtest runner jtreg tests with that option (possibly in a separate RFE).

-------------

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3142