RFR: 8319709: Make GrowableArrayCHeap copyable [v2]

Emanuel Peter epeter at openjdk.org
Mon Dec 4 10:01:39 UTC 2023


On Mon, 4 Dec 2023 09:55:14 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>>> What is the motivation for this? Please add something to JBS. Also see query below.
>>> 
>>> Thanks
>> 
>> I need this feature because I want to store `GrowableArray` s within `GrowableArray`s without an unnecessary pointer indirection. I'll add this justification to JBS.
>
>> @jdksjolen have you checked all use cases where `GrowableArrayCHeap` is copied? I don't know the state of `GrowableArrayCHeap`, but for `GrowableArray`, there are lots of cases where we actually just would like to "swap" or "move" over the data, and not really duplicate/clone the data. It would just be nice to avoid the overhead of more allocations.
>> 
>> I guess that is the drawback of the copy-constructor: you can very easily miss heavy allocations.
>> 
>> Might it be better to forbid the assignment operator and the copy constructor, and make the copying explicit, i.e. with a `copy_from` method?
> 
> Sorry, I forgot to reply last week. I went through all of the usages and nothing does unintended cloning of the data. The  code is written generically for both GA and GACH, so internally unnecessary copying is done (if that's what you're talking about).
> 
> I think `copy_from` is a guard rail that would inconvenience us as it'd be impossible to use the rest of the C++ idioms that come with copy ctr+copy assignment. If you create a copy, then you need to think about the consequences of that copy, regardless of the class. That's just (one of the many) consequences of coding in C++.

@jdksjolen I think the problem is that copy-constructing is also done implicitly, without people realizing it. The question is if we want to trust everybody to understand this and catch it.

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

PR Comment: https://git.openjdk.org/jdk/pull/16559#issuecomment-1838201418


More information about the hotspot-dev mailing list