RFR: 8310743: assert(reserved_rgn != nullptr) failed: Add committed region, No reserved region found

[Albert Mingkun Yang]

On Fri, 30 Jun 2023 13:07:17 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> JDK-8309675 tried to solve the issue of NMT reporting to much committed memory by faking the virtual addresses registered when committing physical memory. This mapping was simply the `virtual address  = physical offset in backing file + heap base`. This works when ZGC gets a contiguous heap which starts at the heap base. However fails if it does not start at the heap base or is discontiguous.
> 
> This proposal fixes this by keeping track of all the reserved virtual address ranges and by creating a mapping from the physical offsets to the actual reserved virtual address ranges. If the physical offsets committed maps to multiple reserved ranges, it is processed as multiple commits.
> 
> The develop flag `ZForceDiscontiguousHeapReservations` is introduced to test the discontiguous heap feature and interactions.
> 
> Testing: 
>  * Added test on all Oracle platforms
>  * GHA
>  * Running tier1-3

src/hotspot/share/gc/z/zNMT.cpp line 44:

> 42:     const size_t reservation_size = _reservations[index]._size;
> 43:     if (*offset_in_reservation < reservation_size) {
> 44:       break;

I wonder if one can return `index` here. Then, after the for-loop, one can write `fatal(...)`.

src/hotspot/share/gc/z/zNMT.cpp line 79:

> 77:         Tracker tracker(Tracker::uncommit);
> 78:         tracker.record((address)sub_range_addr, sub_range_size);
> 79:       }

Sth like `record_virtual_memory_uncommit` would make this more symmetric. (Better be done in a followup PR though.)

src/hotspot/share/gc/z/zNMT.cpp line 91:

> 89:   }
> 90: 
> 91:   assert(left_to_process == 0, "everything was not commited");

When will this be reachable? I'd expect the if-check inside the for-loop to handle exit-condition.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14730#discussion_r1247884226
PR Review Comment: https://git.openjdk.org/jdk/pull/14730#discussion_r1247983800
PR Review Comment: https://git.openjdk.org/jdk/pull/14730#discussion_r1247982053