RFR: 8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap

[Kim Barrett]

[Kim Barret wrote:]
>>> pre-existing: There are a lot of non-static class data members that are pointers to
>>> GrowableArray that seem like they would be better as direct, e.g. non-pointers.
>>> 
>>> pre-existing: There are a lot of iterations over GrowableArray's that would be
>>> simplified by using range-based-for.
>>> 
>>> I'm not a fan of the additional clutter in APIs that the static memory types add.
>>> If we had a variant of GrowableArrayCHeap that was not itself dynamically allocatable
>>> and took a memory type to use internally as a constructor argument, then I think a
>>> lot of that clutter could be eliminated.  It could be used for ordinary data members
>>> that are direct GAs rather than pointers to GAs.  I think there is a way to do something
>>> similar for static data members that are pointers that are dynamically allocated later,
>>> though that probably requires more work.
>>> 
>> 
>> On Fri, 22 Dec 2023 13:22:19 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> FWIW, I added the GrowableArrayCHeap and the static memory type. I did that because there was a perceived need to minimize the memory usage, because we were going to use an extreme amount of these arrays for one of our subsystems in ZGC. It later turned out that we really didn't need to squeeze out the last bit of memory for that use-case. I would really like to get rid of the the static memory type from GrowableArrayCHeap, and just add it as an instance member.

Currently the basic overhead for a static-MEMTYPE GA is 16 bytes (data
pointer, int length and capacity).  So 16 bytes on 64bit platforms.  Adding
the memory type adds 8 bytes (due to alignment), so 50% increase.  Though it
may not matter for dynamically allocated GAs, since std::max_align_t is
probably at least 32 bytes.  Though I think we seriously overuse dynamic
allocation for GAs.

Does it really matter.  I don't know.

StringDedup uses a lot of GAs. (Though it could use 1/2 as many with some work,
since they are used in pairs that always have the same length and capacity. At
the time I was working on it I was feeling lazy and just used pairs of GAs, with the
intention of coming back to that at some point if it proved to be a significant problem.)

> On Dec 22, 2023, at 8:33 AM, Emanuel Peter <epeter at openjdk.org> wrote:
> @stefank Maybe it would then make sense to do that before this change here? Because we will really have to touch all these changes here again for that.
> 
> @kimbarrett @stefank @jdksjolen
> Or alternatively, we just say that we want to keep the C-Heap functionality inside `GrowableArray`, and simply remove `GrowableArrayCHeap`? Though I honestly would prefer a world where every allocation strategy has its own `GrowableArray*` variant, so that it is clear statically that they cannot be assigned to each other. So my preference would even be to have these:
> 
> CHeapGrowableArray
> ArenaGrowableArray
> ResourceAreaGrowableArray
> 
> What do you think?

I think having distinct CHeap, Resource, and Arena allocated types would be an
improvement. If we were using std::vector with associated allocators we'd have
something like that. (The allocator type is a template parameter for
std::vector.) Refactoring GA in that direction is certainly possible, and
might be an improvement.

We could also have two variants of CHeap GAs, one with the memtype being a
constructor argument (and possibly different from the memtype used to
dynamically allocate the GA, as is currently the case, although I think that
feature is never used), and a more compact one with a static memtype.

I did some work on HotSpot allocators for standard containers like std::vector
a while ago that could be applied to GA.  It includes support for dynamic and
static memtypes, and shows that isn't hard to do.

I thought about whether this PR should go ahead without some of that stuff in
place. It touches a lot of places that would probably be touched again if we
went in that sort of direction. OTOH, a lot of these same places would
probably need to be touched repeatedly anyway, one way or another. For
example, dealing with what appears to be a large amount of unnecessary dynamic
allocation of GAs would also touch a lot of these places, and I'm suspicious
of combining the type changes and the allocation changes in one PR.

> And how important is it that we do not use `virtual` with `GrowableArray`? Because the implementation and testing is much nastier without it. Is the V-table still such a overhead that we need to avoid it?

I don't think there is any need for virtual functions in GA.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <https://mail.openjdk.org/pipermail/hotspot-dev/attachments/20231222/4d93430b/signature.asc>