RFR: 8338011: CDS archived heap object support for 64-bit Windows

[Thomas Stuefe]

On Sat, 10 Aug 2024 16:47:32 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/cds/filemap.cpp line 2181:
>> 
>>> 2179:   // for mapped region as it is part of the reserved java heap, which is already recorded.
>>> 2180:   char* addr = (char*)_mapped_heap_memregion.start();
>>> 2181:   char* base = map_memory(_fd, _full_path, r->file_offset(),
>> 
>> So, do I understand this correctly, this always failed on Windows, since we attempt to map into the reserved region of the already existing Java heap?
>
> No, wait, we never would have entered this path since `ArchiveHeapLoader::can_map()` is false on Windows.

But then a follow-up question, what you do now in `map_heap_region_impl` for Windows, would that not be the same as the `ArchiveHeapLoader::load_heap_region` path?

And another question, sorry, unrelated to this PR:

I see we always attempt to load the heap region regardless of here (note how its outside the INCLUDE_CDS_JAVA_HEAP  block):  https://github.com/openjdk/jdk/blob/358d77dafbe0e35d5b20340fccddc0fb8f3db82a/src/hotspot/share/cds/metaspaceShared.cpp#L1200

I wonder whether this is wrong. If it is wrong, its benign? Do we even include heap region in CDS dumps on Windows?

(Sorry for loading this PR with questions)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20514#discussion_r1712673566