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

Kim Barrett kbarrett at openjdk.org
Fri Dec 22 02:25:47 UTC 2023


On Thu, 21 Dec 2023 06:11:03 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/memory/arena.hpp line 209:
>> 
>>> 207: 
>>> 208: #ifdef ASSERT
>>> 209: bool Arena_contains(const Arena* arena, const void* ptr);
>> 
>> This function doesn't seem necessary.  Directly calling arena->contains(ptr) in the one place it's being seems
>> like it should suffice.
>
> @kimbarrett the reason was that I need to call this from the hpp file, and I encountered some circular dependency I did could not resolve. So I needed to move something off to the cpp files. Either I put it in arena.cpp, or in growableArray.cpp. But If I put things off to growableArray.cpp from the GrowableArray class, then it will not easily instantiate the templates, so that is not possible then. Hence I have to put it into arena.cpp

I don't think a global API is warranted to support that local debug-only
implementation detail.

The inclusion problem arises because the PR eliminates GrowableArrayMetadata 
(a non-templated class), thereby forcing the init_checks helper to be moved to
GrowableArray (a class template).  That forced moving the implementation from
the .cpp to the .hpp.

One solution might be to move the new version of init_checks to
growableArray.inline.hpp. After all, breaking circularities is one of the
reasons one might have an inline.hpp file. However, that file doesn't
currently exist, and I think introducing it would have too much fannout. And
it might not even work without even more effort; there might be .hpp files
that contain allocations of GrowableArray.

Much simpler, and I think probably better, is to keep GrowableArrayMetadata
(perhaps under a different name - GrowableArrayArenaHolder?), now holding the
allocation arena and providing init_checks (and anything else that seems
appropriate), with init_checks defined similarly to the current definition in
the .cpp file.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17160#discussion_r1434655989


More information about the graal-dev mailing list