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

David Holmes dholmes at openjdk.java.net
Mon Sep 13 12:46:56 UTC 2021


On Fri, 10 Sep 2021 06:01:42 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> 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++) {
>   ...
> }

I always thought that was a hack way of determining an array size, not an approved method. :(

Don't worry just leave it as-is.

>> 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.

So we are trading off use of constexpr versus broken encapsulation? :( I can't say I understand the details here.

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

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


More information about the hotspot-runtime-dev mailing list