RFR: 8296139: Make GrowableBitMap the base class of all implementations [v2]

[Xin Liu]

On Fri, 4 Nov 2022 13:47:11 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refactor GrowableBitMap using C++ CRTP
>>   
>>   With CPTP, we don't need to have explicit Allocators. Subclasses expose
>>   allocate/free member functions upward to GrowableBitMap<BitMapWithAllocator>. This
>>   aproach is similar to GrowableArray.
>
> src/hotspot/share/utilities/bitMap.hpp line 330:
> 
>> 328:   GrowableBitMap(idx_t size_in_bits, bool clear) : BitMap(reallocate(nullptr, 0, size_in_bits, clear), size_in_bits) {
>> 329:   }
>> 330:   GrowableBitMap() : BitMap(nullptr, 0) {}
> 
> GrowableBitMap(idx_t, bool) is dangerous to use because it calls reallocate, which calls the allocate function of the subclass. This works for ResourceBitMap, but not if you tried to use it for CHeapBitMap or ArenaBitMap. I'd suggest that we never let GrowableBitMap allocate memory in the constructor and instead push that responsiblity down into the subclasses.

agree. I see your patch has changed this. This is safer. 

  GrowableBitMap() : GrowableBitMap(nullptr, 0) {}
  GrowableBitMap(bm_word_t* map, idx_t size_in_bits) : BitMap(map, size_in_bits) {}

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

PR: https://git.openjdk.org/jdk/pull/10941