On Fri, 1 Dec 2023 07:56:04 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
> Before this patch, we always initialized the GrowableArray up to its > `capacity`, and not just up to `length`. This is problematic for a few > reasons: > > - It is not expected. `std::vector` also only initializes the elements up to > its size, and not to capacity. > - It requires a default-constructor for the element type. And the > default-constructor is then used to fill in the elements between length and > capacity. If the elements do any allocation themselves, then this is a waste > of resources. > - The implementation also required the copy-assignment-operator for the > element type. This is a lesser restriction. But the copy-assignment-operator > was used in cases like `append` (where placement copy-construct would be > expected), and not just in true assignment kinds of cases like `at_put`. > > For this reason, I reworked a lot of the methods to ensure that only the > "slots" up to `length` are ever initialized, and the space between `length` > and `capacity` is always garbage. > > ----- > > Also, before this patch, one can CHeap allocate both with `GrowableArray` and > `GrowableArrayCHeap`. This is unnecessary. It required more complex > verification in `GrowableArray` to deal with all cases. And > `GrowableArrayCHeap` is already explicitly a smaller object, and should hence > be preferred. Hence I changed all CHeap allocating cases of `GrowableArray` > to `GrowableArrayCHeap`. This also allows for a clear separation: > - `GrowableArray` only deals with arena / resource area allocation. These are > arrays that are regularly abandoned at the end of their use, rather than > deleted or even cleared. > - `GrowableArrayCHeap` only deals with CHeap allocated memory. We expect that > the destructor for it is called eventually, either when it goes out of scope > or when `delete` is explicitly called. We expect that the elements could be > allocating resources internally, and hence rely on the destructors for the > elements being called, which may free up those internally allocated resources. > > Therefore, we now only allow `GrowableArrayCHeap` to have element types with > non-trivial destructors, but `GrowableArray` checks that element types do not > have non-trivial destructors (since it is common practice to just abandon > arena / resource area allocated arrays, rather than calling the destructor or > clearing the array, which also destructs all elements). This more clearly > separates the two worlds: clean-up your own mess (CHeap) vs abandon your mess > (arena / resource area). > > ----- > > I also completely refactored and improved ... Splitting out part 3 would have been preferable IMO. The CHeap changes are unrelated to the capacity issue and should have their own JBS issue. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16918#issuecomment-1862149245
