RFR: 8271569: Rename cdsoffsets.cpp to cdsConstants.cpp

[Yumin Qi]

On Fri, 10 Sep 2021 01:20:12 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Changed cdsOffsets.cpp to cdsConstants.cpp, now the offsets and constants are initialized static and searched separately.
>> The offsets array could not use 'constexpr' since g++ on MacOs and VSC++ on Windows complained reinterpret_cast in 'offset_of' should not be used in constexpr initialization. Changed some field access for forming global list first.
>> 
>> Note with 'git mv' to rename cdsoffset.cpp to cdsConstants.cpp, same for cdsoffsets.hpp to cdsConstants.hpp, due to the contents changed more than 50% so git will not think cdsConstants.cpp is renamed from cdsoffsets.cpp. Instead, it is regarded as a new file.
>> 
>> Tests: ter1-4
>> 
>> Thanks
>> Yumin
>
> src/hotspot/share/cds/cdsConstants.cpp line 61:
> 
>> 59: 
>> 60: size_t get_cds_offset(const char* name) {
>> 61:   for (int i = 0; i < (int)(sizeof(cds_offsets)/sizeof(cds_offsets[0])); i++) {
> 
> Can't we just define a size for the array and use that? It isn't really a dynamic/unknown quantity.

The array size is known at compile time --- it is not dynamic but we do not need to manually count the size of the array this way. If define a size for the array, we need to count the number of items in the array to define the array size. We may add more items to the array --- just add items and don't need change anything else. Since total array memory size and array component size are known at compile time, so compiler will not generate code to calculate the size. How about this? 
const int size =   (int)(sizeof(cds_offsets)/sizeof(cds_offsets[0]));
for (int i = 0; i < size; i++) {
  ...
}

> src/hotspot/share/cds/dynamicArchive.hpp line 45:
> 
>> 43: 
>> 44: public:
>> 45:   int _base_region_crc[MetaspaceShared::n_regions];
> 
> This looks like it should be private with either a public accessor or else a friend declaration for the client code.

Let me try but I remember it does not work with constexpr initialization. This is why I move the data field to public not using the public accessor.

> src/hotspot/share/cds/filemap.hpp line 211:
> 
>> 209:   // will function correctly with this JVM and the bootclasspath it's
>> 210:   // invoked with.
>> 211: public:
> 
> Again this looks like it should be private with either a public accessor or else a friend declaration for the client code.

same reason as above.

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

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