RFR: 8281946: VM.native_memory should report size of shareable memory [v8]
Matias Saavedra Silva
matsaave at openjdk.org
Fri Dec 9 16:00:57 UTC 2022
On Fri, 9 Dec 2022 07:34:42 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Encapsulated readonly into CDS
>
> 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.
Thank you for the feedback! I agree that NMT should be blind to these features but I'm not entirely sure the best way to pass on the readonly amount without at least having FileMapInfo. Maybe there should be an RFE for the minimal header and then some cleanup?
-------------
PR: https://git.openjdk.org/jdk/pull/11401
More information about the hotspot-runtime-dev
mailing list