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

[Gerard Ziemski]

On Tue, 23 May 2023 09:40:45 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:
>> 
>>   Johan's feedback: add comments, handle pesky NULL
>
> src/hotspot/share/services/virtualMemoryTracker.cpp line 533:
> 
>> 531:     VirtualMemorySummary::record_released_memory(size, reserved_rgn->flag());
>> 532:     assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
>> 533:     if (reserved_rgn->base() == addr || reserved_rgn->end() == addr + size) {
> 
> This is the one I wanted a comment for! "If the two regions start at the same place, or ends at the same place, then we can shrink the region from the left or right. Otherwise, we'll have to split it up"

We still have to split in some cases where the two regions start at the same place, i.e.:


aaaa bbbb cccc
xxxx xxxx


Where a,b,c are the underlaying strips, and x denotes the chunk we want to free.

> src/hotspot/share/services/virtualMemoryTracker.cpp line 534:
> 
>> 532:     assert(reserved_rgn->contain_region(addr, size), "Not completely contained");
>> 533:     if (reserved_rgn->base() == addr || reserved_rgn->end() == addr + size) {
>> 534:       reserved_rgn->exclude_region(addr, size);
> 
> This is unfortunately not named "shrink_region_from" or something similar :-(. I don't expect you to rename the function, to be clear :).

I'm not 100% liking the name "shrink_region_from", but if we were to find one that we can all agree on, I can factor out that code into its own method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13813#discussion_r1204576631
PR Review Comment: https://git.openjdk.org/jdk/pull/13813#discussion_r1204582431