RFR: 8365245: Move size reducing operations to GrowableArrayWithAllocator
Kim Barrett
kbarrett at openjdk.org
Mon Aug 11 13:14:03 UTC 2025
On Mon, 11 Aug 2025 13:06:01 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
> Please review this change to GrowableArray to move size reducing operations
> from GrowableArrayBase and GrowableArrayView to GrowableArrayWithAllocator.
> See JBS for rationale for this move.
>
> This is mostly just a simple code motion change, with the moved functions
> being updated to account for different name lookup rules now that they are in
> a class with a class template base class. For the most part this refactoring
> didn't require any client code changes. All but one use of one moved function
> was with a GrowableArrayWithAllocator-derived class, so not affected by moving
> the functions. The one exception was a test of ZArraySlice that was modified
> to no longer use GA::pop().
>
> Testing: mach5 tier1.
I didn't make any code changes to address some issues noticed while doing this
refactoring. Some things I noticed and will file JBS issues for were:
- GrowableArray::remove_range doesn't allow an empty range. Wondering why, as
that's potentially inconvenient and contrary to usual practice.
- GrowableArray::appendAll effectively does a bunch of push_backs, which can
lead to multiple reallocations. It should expand capacity once up front, based
on the size of the array being appended.
- GrowableArray::remove_if_existing searches for the the item twice, once to
detect it's presence (using `contains`), and again as part of `remove`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/26726#issuecomment-3174744007
More information about the hotspot-dev
mailing list