RFR: 8254012: NMT: MetaspaceSnapshot::snapshot uses wrong enum

[Thomas Stuefe]

On Wed, 14 Oct 2020 07:46:26 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> Please review this simple change.
>> Snapshot should be done first on NonClassType which will include ClassType allocation when
>> Metaspace::using_class_space() is false.  _class_vsm (a VirtualSpaceManager) is set only for class type allocation when
>> using_class_space is true. The correct order is do snapshot for NonClassType first, then for ClassType if
>> using_class_space.  Tests: mach5 tier1-4 in progress.
>
> The change looks good // not Reviewer though.
> Thanks for fixing!

> > > I wonder why this bug has not been discovered until now. It seems like the old code would not work at all when
> > > compressed class space is disabled. Do we have any test cases for it? Or, is this feature actually used?
> > 
> > 
> > MetaspaceSnapshot predefine the _reserved_in_bytes (and other two, _commited_in_byte, _used_in_bytes) with dimension of
> > MetadataTypeCount, the array of index ClassType is a valid slot. In this bug, if CCP is disabled, the snapshot for
> > ClassType gets updated with zero, but non class type data is not update in snapshot. There is no output for class type
> > data when CCP is disabled. While the original data recording is correct (at allocation) the snapshot did not get it. I
> > will do some tests.
> 
> This is confusing to be sure. The part that is broken by this bug is using the basline..diff functionality if
> compressed class space is off. All other metaspace numbers are still okay because they are printed via other code
> paths.  Reproduction:
> 
> ```
> >java -XX:-UseCompressedClassPointers -XX:NativeMemoryTracking=summary de.stuefe.repros.metaspace.JustLoadAndLoad &
> > jcmd JustLoadAndLoad VM.native_memory baseline
> 7518:
> Baseline succeeded
> > jcmd JustLoadAndLoad VM.native_memory summary.diff
> 7518:
> 
> Native Memory Tracking:
> 
> Total: reserved=5015640KB +163200KB, committed=543088KB +161924KB
> <skip>
> -                     Class (reserved=427146KB +161234KB, committed=425458KB +159954KB)
>                             (classes #6011 +1185)
>                             (  instance classes #5706 +1185, array classes #305)
>                             (malloc=9354KB +3538KB #41433 +14309)
>                             (mmap: reserved=417792KB +157696KB, committed=416104KB +156416KB)
>                             (  Metadata:   )   << no increase shown
>                             (    reserved=0KB, committed=0KB)
>                             (    used=0KB)
>                             (    free=0KB)
>                             (    waste=0KB =-nan%)
> ```
> 
> And no, there are no tests for that. This is an old bug.

Not sure how Oracle handles this usually, but maybe it makes sense to reformulate the JBS issue and describe above
effect? Or, alternatively, open a new one as a duplicate to this bug?

When people look into the JBS for this specific NMT bug they may not notice the harmless test error.

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

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