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