RFR: 8279018: CRC calculation in CDS should not include _version and _head_size [v2]
Yumin Qi
minqi at openjdk.java.net
Wed Dec 22 00:46:18 UTC 2021
On Tue, 21 Dec 2021 22:19:15 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
>> 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
>
> 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?
We can return different values if INCLUDE_CDS is not defined. The functions should be available for java native functions, or a link error will be thrown when they are linked. Same answer to your next question.
> 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"
I will add include "cds.h". Those including, should be cleaned up in a separate issue.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6899
More information about the hotspot-runtime-dev
mailing list