RFR: 8279018: CRC calculation in CDS should not include _version and _head_size [v2]

Calvin Cheung ccheung at openjdk.java.net
Tue Dec 21 22:23:16 UTC 2021


On Tue, 21 Dec 2021 04:17:50 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Please review the simple change for not including _version and _header_size in crc computation. These two fields are checked separately and they should not affect crc.
>> 
>> tests: tier1,tier4
>> 
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Restore GenericCDSFileMapHeader data layout, change compute_crc instead, add code to WhiteBox to get versions instead of hard code

I have couple of comments in whitebox.cpp.

src/hotspot/share/prims/whitebox.cpp line 1972:

> 1970:   return (jint)CURRENT_CDS_ARCHIVE_VERSION;
> 1971: WB_END
> 1972: 

Should these two functions be inside a `#if INCLUDE_CDS` block?

src/hotspot/share/prims/whitebox.cpp line 2696:

> 2694:                                                       (void*)&WB_GetDefaultArchivePath},
> 2695:   {CC"getCDSGenericHeaderMinVersion",     CC"()I",    (void*)&WB_GetCDSGenericHeaderMinVersion},
> 2696:   {CC"getCurrentCDSVersion",              CC"()I",    (void*)&WB_GetCDSCurrentVersion},

I think the above should also be inside a `#if INCLUDE_CDS` block.

Also, I think the cds.h should be included explicitly; currently it is included transitively via filemap.hpp.

Pre-existing issue: below CDS specific include statements should be inside a `#if INCLUDE_CDS` block.

  27 #include "cds/cdsConstants.hpp"
  28 #include "cds/filemap.hpp"
  29 #include "cds/heapShared.inline.hpp"
  30 #include "cds/metaspaceShared.hpp"

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

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


More information about the hotspot-runtime-dev mailing list