[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 <stef...@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 <epe...@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.
signature.asc
Description: Message signed with OpenPGP
