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

[Volker Simonis]

On Sat, 5 Nov 2022 00:53:51 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> BitMap is an abstract class. All subclasses provide their own allocators. It is the allocator brings the capability to 'resize' the container. 
>> I would like to add a new 'GrowableBitmap' and make it the common base class. 
>> 
>> To substitute 'VectorSet', we also need to merge ResourceBitMap and ArenaBitMap because VectorSet supports Resource and Arena storage at the same time. 
>> This patch unifies them.  ResourceBitMap is a specialized ArenaBitMap whose allocator is with NULL arena.
>> 
>> All subclasses of GrowableBitMap are elastic. I will add new interface like VectorSet::test_set(index). It will grow to include the new index.
>
> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge branch 'master' into JDK-8296139
>  - Bring back verify_size to the constructor of BitMap.
>    
>    Also resume the underlying member data to be private. Subclasses of
>    GrowableBitMap update them using update(map, size).
>  - Reivew comments from stefank
>  - 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.
>  - Add unittest for Arena.
>  - Avoid polluting namespace.
>    
>    uses qualified bm_word_t and idx_t.
>  - Change ResourceBitMap to subclass of ArenaBitMap.
>  - Add GrowableBitMap.

In general looks good. I have a single, small comment. Once we resolve that, I'll approve the change.

src/hotspot/share/utilities/bitMap.cpp line 637:

> 635: template class GrowableBitMap<ArenaBitMap>;
> 636: template class GrowableBitMap<ResourceBitMap>;
> 637: template class GrowableBitMap<CHeapBitMap>;

Do you really need this explicit template instantiations? I think such code is only required in libraries where you want to be sure to have specific instantiations even though they are not used within the library itself. But here you will get these instantiations anyway whenever `ArenaBitMap`, `ResourceBitMap` or `CHeapBitMap` is used because they are derived from `GrowableBitMap<ArenaBitMap>`, `GrowableBitMap<ResourceBitMap>` and `GrowableBitMap<CHeapBitMap>` respectively.

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

Changes requested by simonis (Reviewer).

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