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