RFR: 8287135: Calculation of jmm_GetMemoryUsage is wrong [v5]

[Thomas Stuefe]

On Mon, 27 Jun 2022 08:15:01 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> It seems that calculation of MemoryMXBean.getNonHeapMemoryUsage(jmm_GetMemoryUsage) is wrong.
>> 
>> Currently, `NonHeapUsage=CodeCache+Metaspace(ClassTypeSpace+NonClassTypeSpace)+CompressedClassSpace(ClassTypeSpace)`
>> 
>> ==> CodeHeap 'non-nmethods' 1532544 (Used)
>> ==> CodeHeap 'profiled nmethods' 0
>> ==> CodeHeap 'non-profiled nmethods' 13952
>> ==> Metaspace 506696
>> ==> Compressed Class Space 43312
>> init = 7667712(7488K) used = 2096504(2047K) committed = 8454144(8256K) max = -1(-1K)
>> 
>> In this way, getNonHeapMemoryUsage is larger than it ought to be, it should be `NonHeapUsage = CodeCache + Metaspace`.
>
> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   address @tstuefe's comments; remove unnecessary declaration

I think @iklam meant something somewhat different. Since we remove the class space bean, we now need to rewrite those tests using that bean.

Lets say we create the following whitebox APIs

- long WB_getClassSpaceUsage
- long WB_getClassSpaceCommitted

Maybe also (to be symmetric, and to not have to mix bean code and this code in test/hotspot/jtreg/vmTestbase/metaspace/gc/MemoryUsageTest.java:
- long WB_getMetaSpaceUsage
- long WB_getMetaSpaceCommitted

Maybe also:
- boolean WB_usesClassSpace()
- long WB_classSpaceMax()
though these can easily be implemented using existing whitebox method (there is one to get VM flags, so you could get   `UseCompressedClassPointers` and `CompressedClassSpaceSize` instead.

Now:

- In test/hotspot/jtreg/vmTestbase/metaspace/gc/MemoryUsageTest.java, you don't use beans anymore but those functions. You'd rewrite "getUsed()" and "getCommitted()" to use the new whitebox functions.

- In metaspce/ShrinkGrowTest, the bean is only used to confirm that class space is active. You can use WB_usesClassSpace for that, or the value of "UseCompressedClassPointers" flag.

- In test/hotspot/jtreg/vmTestbase/vm/mlvm/indy/stress/gc/lotsOfCallSites/Test.java, we use Compressed Class Bean to get Compressed Class usage and maximum size. YHere, you would have to exchange `classMetadataPoolMXB.getUsage();` with `WB_getClassSpaceUsage()` and `classMetadataPoolMXB.getUsage();` with `CompressedClassSpaceSize`

----

Note that since this RFR really grew in complexity compared to what you wanted to do originally, I'd be fine with dropping it altogether. If we continue, I think above proposal is the simplest way to go.

src/hotspot/share/services/memoryPool.cpp line 188:

> 186: 
> 187: // Note, any NonHeap pools would be counted into Non-Heap memory by MemoryMXBean.getNonHeapMemoryUsage.
> 188: // Make sure that it behaves as you expected if you want to add a new type of NonHeap pool

This comment is somewhat cryptic. Behave like what?

src/hotspot/share/services/memoryPool.cpp line 189:

> 187: // Note, any NonHeap pools would be counted into Non-Heap memory by MemoryMXBean.getNonHeapMemoryUsage.
> 188: // Make sure that it behaves as you expected if you want to add a new type of NonHeap pool
> 189: MetaspacePool::MetaspacePool() :

Can you please add a comment like
"Accounts for Metaspace (both class space and non-class space).
 Max usage is MaxMetaspaceSize. Current usage includes prematurely freed blocks.
 As far as this API is concerned, CompressedClassSpaceSize is ignored."

test/hotspot/jtreg/vmTestbase/nsk/share/gc/gp/GarbageUtils.java line 53:

> 51:             ANY (),
> 52:             HEAP("Java heap space"),
> 53:             METASPACE("Metaspace");

This chance is wrong, I think. We still have class space OOMEs.

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

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.org/jdk/pull/8831