RFR: 8319115: GrowableArray: Do not initialize up to capacity
Emanuel Peter
epeter at openjdk.org
Wed Dec 20 12:58:53 UTC 2023
On Mon, 18 Dec 2023 22:48:18 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> @eme64 Is it feasible to split this up to solve each of the problems you identify in stages? There is also overlap here with JDK-8319709 IIUC. Thanks.
>
>> @dholmes-ora These are the "parts":
>>
>> 1. initialize up to capacity vs length
>>
>> 2. update the test to verify this (complete refactoring)
>>
>> 3. remove cheap use of GrowableArray -> use GrowableArrayCHeap instead
>>
>>
>> The first 2 items are inseparable, I cannot make substantial changes to many GrowableArray methods without there even being tests for them. And the tests would not pass before the changes for item 1, since the tests also verify what elements of the array are initialized. So adding the tests first would not be very feasible.
>>
>> The 3rd item could maybe be split, and be done before the rest. Though it would also require lots of changes to the test, which then I would have to completely refactor with items 1+2 anyway.
>>
>> And the items are related conceptually, that is why I would felt ok pushing them together. It is all about when (item 1) and what kinds of (item 3) constructors / destructors are called for the elements of the arrays, and verifying that thoroughly (item 2).
>>
>> Hence: feasible probably, but lots of work overhead. Do you think it is worth it?
>
> I too would prefer that it be split up. It's very easy to miss important details in amongst all the mostly relatively
> simple renamings. That is, I think 3 should be separate from the other changes.
@kimbarrett @dholmes-ora I just published this: https://github.com/openjdk/jdk/pull/17160
It removes the C-Heap capability from `GrowableArray`, and replaces usages with `GrowableArrayCHeap`.
Bonus: we can now check that all element types of `GrowableArray` should be trivially destructible (that way it is always ok to abandon elements on the array, when the arena or ResourceMark go out of scope).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16918#issuecomment-1864429000
More information about the graal-dev
mailing list