On Thu, 21 Dec 2023 10:48:30 GMT, Johan Sjölen <jsjo...@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/utilities/growableArray.hpp line 621:
> 
>> 619: //  - Arena
>> 620: //
>> 621: // Itself, it can be embedded, on stack, resource_arena or arena 
>> allocated.
> 
> "Itself can be allocated on stack, resource area or arena allocated."

That it can be embedded into another class/struct is a given, imho.

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

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

Reply via email to