On Mon, 18 Dec 2023 22:48:18 GMT, Kim Barrett <kbarr...@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
