RFR: 8296139: Make GrowableBitMap the base class of all implementations

[Axel Boldt-Christmas]

On Wed, 2 Nov 2022 05:54:27 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.

I am curious how the final type hierarchy will look here. 

I am guessing the `GrowableBitMap` will get something like `set_bit_grow` etc. 
Given that more than just `resize` and `[re]initialize` will now change the underlying storage maybe we should have the same/similar abstraction as `GrowableArray`. 
At least the separating the code that does not change the size and is allocator agnostic (like `GrowableArrayView`) and the code which may grow and deallocate (like `GrowableArrayWithAllocator`). 

Other than questions of where this is going and the resource/arena thing this looks good to me, and I like the separation of the allocation logic and the pure bitmap logic.

P.S. Maybe someone more knowledgable than I can comment on this style of using allocators vs the CRTP style used in `GrowableArray`. The CRTP seems more encapsulating to me.

src/hotspot/share/utilities/bitMap.hpp line 412:

> 410: 
> 411: // A BitMap with storage in a ResourceArea. It is an ArenaBitMap but _arena is nullptr;
> 412: class ResourceBitMap : public ArenaBitMap {

How do we want to treat resource allocations which look like arena allocations? The compiler code does have a places where objects are arena allocated in `Thread::current()->resource_area()`. I am not sure about having this behaviour in the type system. 

The places where `VectorSet()` is used today, can we not just use `ArenaBitMap(Thread::current()->resource_area(), [...])` instead. And have the replacement be `VectorSet` -> `ArenaBitMap`

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

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