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

[Thomas Stuefe]

On Sun, 11 Aug 2024 21:11:31 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> 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)
>
>> 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?
> 
> No. `ArchiveHeapLoader::can_load()` and `ArchiveHeapLoader::can_map()` are not capabilities of the platform, but capabilities of the GC. 
> 
> - `can_map()` is hard-coded for G1.
> - `can_load()` returns `Universe::heap()->can_load_archived_objects() && UseCompressedOops`. This returns true only on Serial, Parallel and Epsilon.
> 
> So on Windows, when G1 is used, we go into this funny "mapping" mode except that the "mapping" is really implemented with `os::read()`.
> 
>> 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)
> 
> It's benign. If the heap region is not in the CDS archive, the `static_mapinfo->map_or_load_heap_region()` call does nothing.

Thank you for explaining.

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

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