RFR: 8281946: VM.native_memory should report size of shareable memory [v8]

[Thomas Stuefe]

On Thu, 8 Dec 2022 23:04:28 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> The native memory reporting has been improved to also include the amount of shareable (read only) data. Verified with test tiers 1,2, and 4.
>> 
>> Sample output:
>> 
>> Total: reserved=33890498670, committed=2360907886
>>        malloc: 75159662 #26118
>>        mmap:   reserved=33815339008, committed=2285748224
>> .............
>> -        Shared class space (reserved=16777216, committed=13545472, readonly=8315472)
>>                             (mmap: reserved=16777216, committed=13545472) 
>>  
>> -               Arena Chunk (reserved=441792, committed=441792)
>>                             (malloc=441792)
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Encapsulated readonly into CDS

Hi Matias,

we are getting there... see comment inline.

Cheers, Thomas

src/hotspot/share/services/memReporter.cpp line 179:

> 177:   FileMapInfo::current_info() != nullptr ? read_only_bytes = FileMapInfo::current_info()->readonly_total() : read_only_bytes = 0; // static archive
> 178:   if (FileMapInfo::dynamic_info() != nullptr)
> 179:     read_only_bytes += FileMapInfo::dynamic_info()->readonly_total(); // dynamic archive

Still does not belong here in this detail... NMT should not have to know that there are multiple FileMapInfo; it should not even have to know what a FileMapInfo is.

Proposal: Merge this into `FileMapInfo::readonly_total()`, make `FileMapInfo::readonly_total()` static, adapt comment (comment above about Windows details belongs atop of the prototype). And here, we just want to have a "const size_t readonly = FileMapInfo::readonly_total".

Ideally, it would be cool had CDS an outward facing minimal header (e.g. cdsStatistics.hpp) with APIs like these (e.g. `static size_t CDS::readonly_total()`. Because we still need to include the rather fat header fileMapInfo.hpp.

src/hotspot/share/services/memReporter.hpp line 28:

> 26: #define SHARE_SERVICES_MEMREPORTER_HPP
> 27: 
> 28: #include "cds/filemap.hpp"

Not necessary in the header. Please move to the cpp file.

src/hotspot/share/services/memReporter.hpp line 81:

> 79:   size_t reserved_total(const MallocMemory* malloc, const VirtualMemory* vm) const;
> 80:   size_t committed_total(const MallocMemory* malloc, const VirtualMemory* vm) const;
> 81:   size_t readonly_total(FileMapInfo* info) const;

Not necessary

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

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