RFR: 8319117: GrowableArray: Allow for custom initializer instead of copy constructor [v8]

Johan Sjölen jsjolen at openjdk.org
Wed Nov 1 15:26:08 UTC 2023


On Wed, 1 Nov 2023 14:04:23 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Call dtr and global placement new
>>  - Do not add this
>
> src/hotspot/share/utilities/growableArray.hpp line 414:
> 
>> 412:         ::new (&this->_data[j]) E(args...);
>> 413:       }
>> 414:       this->_len = i + 1;
> 
> `grow()` will fill the range `[0,_len)` with copies of the current array contents, and `[_len,_capacity)` with
> default constructed values.  Then we come here and replace the contents from `[_len,_capacity)`.  That
> seems wasteful.  Maybe the args need to be passed through to `grow` and so to `expand_to`?

Hi,

I have this PR which is meant to simply do away with `[len, cap)` being initialized: https://github.com/openjdk/jdk/pull/16418

> src/hotspot/share/utilities/growableArray.hpp line 420:
> 
>> 418: 
>> 419:   template<typename... Args>
>> 420:   void at_put_grow(int i, const E& elem, const Args&... args) {
> 
> I don't understand the point of this change, as the old code defaulted fill object seems like it should be fine
> for at_put_grow, which requires the element type to be copyable/assignable anyway to store the new element
> value.

Sorry, I'm not sure what you're missing here. This change enables in-place construction of the objects and the `Args...` doesn't have to be of type `E`. Is the issue with the default being removed?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16409#discussion_r1378943694
PR Review Comment: https://git.openjdk.org/jdk/pull/16409#discussion_r1378946744


More information about the hotspot-dev mailing list