RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

Emanuel Peter epeter at openjdk.org
Thu Dec 21 06:13:48 UTC 2023


On Wed, 20 Dec 2023 20:35:05 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> [JDK-8247755](https://bugs.openjdk.org/browse/JDK-8247755) introduced the `GrowableArrayCHeap`. This duplicates the current C-Heap allocation capability in `GrowableArray`. I now remove that from `GrowableArray` and move all usages to `GrowableArrayCHeap`.
>> 
>> This has a few advantages:
>> - Clear separation between arena (and resource area) allocating array and C-heap allocating array.
>> - We can prevent assigning / copying between arrays of different allocation strategies already at compile time, and not only with asserts at runtime.
>> - We should not have multiple implementations of the same thing (C-Heap backed array).
>> - `GrowableArrayCHeap` is NONCOPYABLE. This is a nice restriction, we now know that C-Heap backed arrays do not get copied unknowingly.
>> 
>> **Bonus**
>> We can now restrict `GrowableArray` element type `E` to be `std::is_trivially_destructible<E>::value == true`. The idea is that arena / resource allocated arrays get abandoned, often without being even cleared. Hence, the elements in the array are never destructed. But if we only use elements that are trivially destructible, then it makes no difference if the destructors are ever called, or the elements simply abandoned.
>> 
>> For `GrowableArrayCHeap`, we expect that the user eventually calls the destructor for the array, which in turn calls the destructors of the remaining elements. Hence, it is up to the user to ensure the cleanup. And so we can allow non-trivial destructors.
>> 
>> **Testing**
>> Tier1-3 + stress testing: pending
>
> src/hotspot/share/memory/arena.hpp line 209:
> 
>> 207: 
>> 208: #ifdef ASSERT
>> 209: bool Arena_contains(const Arena* arena, const void* ptr);
> 
> This function doesn't seem necessary.  Directly calling arena->contains(ptr) in the one place it's being seems
> like it should suffice.

@kimbarrett the reason was that I need to call this from the hpp file, and I encountered some circular dependency I did could not resolve. So I needed to move something off to the cpp files. Either I put it in arena.cpp, or in growableArray.cpp. But If I put things off to growableArray.cpp from the GrowableArray class, then it will not easily instantiate the templates, so that is not possible then. Hence I have to put it into arena.cpp

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17160#discussion_r1433582008


More information about the graal-dev mailing list