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

[Xin Liu]

On Wed, 2 Nov 2022 14:02:09 GMT, Axel Boldt-Christmas <aboldtch 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.
>
> 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`

This is how I treat them using the base pointer 'ArenaBitMap*'
https://github.com/navyxliu/jdk/commit/cdbc0e1c97f747fd53ad408d59e65ec8adf0fc16#diff-17a9d48a5570e5bcde557c8d1efcbe78d69a5657bc865bed37b22c3c8b5d447d

> 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

`Thread::current()->resource_area()` returns ResourceArena*. ResourceArena is a subclass of Arena. In this sense, we can use the same ArenaAllocator to handle both ResourceBitMap and ArenaBitMap. That's why I remove 'ResourceAllocator' in this patch. 

I think it is possible to do what you said. Let's say we use ArenaBitMap as 'ResourceBitMap'. Shall we keep ArenaBitMap and ResourceBitMap separated then? In classic object-oriented design, ResourceBitMap 'is-a' ArenaBitMap, isn't it? If we keep 2 classes in parallel, we may end up 2 'ResourceBitMap' with different behaviors. 

In current patch, ResourceBitMap is just an adapter. It supports copy constructor and copy assignment because 'resource arena' is unique.

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

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