RFR: 8255917: runtime/cds/SharedBaseAddress.java failed "assert(reserved_rgn != 0LL) failed: No reserved region" [v3]

Thomas Stuefe stuefe at openjdk.java.net
Tue Dec 15 09:46:59 UTC 2020


On Mon, 14 Dec 2020 20:12:06 GMT, Yumin Qi <minqi at openjdk.org> wrote:

> Hi, Thomas
> 
> > I don't understand the special handling in NMT.
> > Unix: We reserve a region, split it, tell NMT about the split (see posix version of os::split_reserved_memory), then later release both parts separately.
> > There is no problem for Linux/Unix here since split does not do the split.
> > Windows:
> > 
> > 1. use_archive_base_addr = true: we allocate the spaces separately, later release them separately
> >    Yes, the two spaces are released separately and NMT will find exact matched regions to remove.
> > 2. use_archive_base_addr = false: we allocate total space, leave it unsplit, then later release the total space.
> 
> The space reserved as a whole, and we did not do the 'split' on it. The release should be on whole too. When unmapping regions, the released on region in fact did not do anything in nmt since it is within the archive space. Since the new code is release on the whole space (there is no call to release on class_space_rs in this case), so it check if it is > archive_space_rs (with tag of mtClassShared) we know it is for releasing the whole space. The space which is bigger than archive_space_rs with tag of mtClassShared should be only the whole space.

Okay. Lets see if I understand:

For (2) and for Unix, we reserve the total area. Then we tell NMT to semantically split it up in two regions (`MemTracker::record_virtual_memory_split_reserved...`) - later we tag the first one with "mtClassShared", the second with "mtClass". Then, if initializing the archive fails, we release now the total area. Since NMT has only the split view, it finds the mtClassShared entry at base address which is of smaller size. We run into the `if (size > reserved_rgn->size()) {` case in `virtualMemoryTracker.cpp:517`. We release both separately.

Right?

Also: the section at `virtualMemoryTracker.cpp:509` `if (reserved_rgn->contain_region(addr, size)) {` should only be needed for Posix, right? We never should run into the problem that we unmap a section within another section on Windows? So, in theory, we should be able to make this a `WINDOWS_ONLY(ShouldNotReachHere())` ?

--

Going forward, I wonder whether NMT should have general support for releasing multiple mappings in one go. Since we introduced the concept of "artificial split" with `MemTracker::record_virtual_memory_split_reserved...` we may want to have now a corresponding "multi-release". Or, Idk, maybe just remove the `MemTracker::record_virtual_memory_split_reserved` and the corresponding code in NMT release? and just live with the fact that for a "shallow split" the numbers in NMT are accounted to the wrong tag (mtClassShared gets mtClass). 

> 
> > Where is this new NMT coding needed?
> 
> The code is to make the bytes in snapshot correct. That, when all the spaces release, the bytes in the reserved slot should be 'zero'.
> 
> Thanks for review.

The rest looks good AFAICS. 

For JDK17, I really hope we can simplify and streamline this coding. Maybe if we consider one or two things:
- just do File IO always on Windows? I did make a test and I may be wrong but I could not measure any startup performance loss compared to the multiple mappings.
- instead of many little mmap calls, just mmap the whole archive right at reservation time alongside the class space reservation. That would increase the chance that everything goes well, and reduce the complexity of the cleanup if it doesn't.

What do you think, does that make sense?

Thanks, Thomas

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

PR: https://git.openjdk.java.net/jdk/pull/1750


More information about the hotspot-dev mailing list