On Fri, 5 Sep 2025 08:33:38 GMT, Stefan Johansson <[email protected]> wrote:

>> Please review this enhancement to the smaps parser and printer.
>> 
>> **Summary**
>> While working on [JDK-8366434](https://bugs.openjdk.org/browse/JDK-8366434) 
>> one idea for a test to verify if the heap CAN be backed by transparent huge 
>> pages (THP) was to use PrintMemoryMapAtExit and look at the tags printed for 
>> JAVAHEAP.
>> 
>> It turns out this worked in most cases but on systems where the THP mode is 
>> configured as 'always' but there are no huge pages available for use, the 
>> JAVAHEAP line will not show any indication that it can be backed by huge 
>> pages.
>> 
>> For this to be possible we need to parse the field THPeligible as well and 
>> include this information in the printouts. With this change we now parse the 
>> THPeligible field and tag mapping that are THP eligible with `thpel`. We 
>> skip this tag for mappings that are tagged with `thpad` (madvised with 
>> MADV_HUGEPAGE) to avoid too much THP info on one mapping.
>> 
>> **Testing**
>> * Mach5 testing with the existing test showed a need to update the regex for 
>> verifying Metaspace and the fix have been tested manually locally as well as 
>> in Mach5.
>
> Stefan Johansson has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Offline comments and discussion

Sorry for the delay, I was missing this one.

Good. Small nits inside, up to you if you take the suggestions. I don't need 
another review.

src/hotspot/os/linux/procMapsParser.cpp line 83:

> 81:   // scan THPeligible into a bool
> 82:   int thpel = 0;
> 83:   if (::sscanf(_line, "THPeligible: %d", &thpel) == 1) {

Suggestion:

  if (::sscanf(_line, "THPeligible: %d", &thpel) == 1) {
    assert(thpel == 1 || thpel == 0, "Unexpected value %d", thpel);

test/hotspot/jtreg/serviceability/dcmd/vm/SystemMapTestBase.java line 102:

> 100:             // metaspace
> 101:             regexBase_committed + "META.*",
> 102:             // parts of metaspace should be uncommitted, those parts can 
> be thp eligible

Suggestion:

            // parts of metaspace should be uncommitted, those parts may or may 
not be be thp-eligible

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27098#pullrequestreview-3240000359
PR Review Comment: https://git.openjdk.org/jdk/pull/27098#discussion_r2359510679
PR Review Comment: https://git.openjdk.org/jdk/pull/27098#discussion_r2359515522

Reply via email to