RFR: 8263464: NMT: assert in gtest os.release_multi_mappings_vm [v6]

[Gerard Ziemski]

On Fri, 19 May 2023 11:40:42 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use size_t instead long
>
> src/hotspot/share/services/virtualMemoryTracker.cpp line 531:
> 
>> 529:     VirtualMemorySummary::record_released_memory(size, reserved_rgn->flag());
>> 530:     assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
>> 531:     if (reserved_rgn->base() == addr || reserved_rgn->end() == addr + size) {
> 
> Can you assign the condition a bool variable with a descriptive name or have a comment? Something such as `bool not_in_middle` or `// Can only exclude if not from middle`

Added comments

> src/hotspot/share/services/virtualMemoryTracker.cpp line 550:
> 
>> 548:       }
>> 549:     }
>> 550:   } else {
> 
> `if (size <= reserved_rgn->size() && reserved_rgn->contain_region(addr, size))` always returns, so we don't need an `else` branch here, right?

This branch will indeed be taken in the multiple regions case (i.e. the synthetic region that we can just  make up, that doesn't align with underlaying real regions and spawns across multiple of them)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13813#discussion_r1199172979
PR Review Comment: https://git.openjdk.org/jdk/pull/13813#discussion_r1199172800