RFR: 8321033: Avoid casting Array to GrowableArray

Kim Barrett kbarrett at openjdk.org
Wed Jun 5 04:17:57 UTC 2024


On Tue, 4 Jun 2024 20:49:16 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

> `ModuleEntry::_reads` is declared as an GrowableArray<ModuleEntry*>*, but when stored in a CDS archive, it's assigned to an Array<ModuleEntry*>*. To ensure better type safety, `ModuleEntry::_reads` is changed to a generic pointer which uses two different getters and setters as well as two booleans to ensure the value is interpreted correctly. This was chosen to avoid introducing a new field to the ModuleEntry class, as another pointer and further alignment would increase the size of the ModuleEntry array. Verified with tier1-5 tests.

Changes requested by kbarrett (Reviewer).

src/hotspot/share/classfile/moduleEntry.cpp line 473:

> 471:     if (_reads == nullptr) { _reads_is_growable = true; }
> 472:   )
> 473:   set_archived_reads(write_growable_array(reads()));

I don't really like this forcing of the flag to make the call to reads() happy.  Maybe something like the
following instead?

GrowableArray<ModuleEntry*>* entries = nullptr;
if (_reads != nullptr) { entries = reads(); }
set_archived_reads(write_growable_array(entries));

Similarly in load_from_archive with the other flag.

src/hotspot/share/classfile/moduleEntry.hpp line 73:

> 71:   void* _reads; // list of modules that are readable by this module
> 72:   DEBUG_ONLY(bool _reads_is_growable);
> 73:   DEBUG_ONLY(bool _reads_is_archived);

These new members should be initialized in the constructor.

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

PR Review: https://git.openjdk.org/jdk/pull/19549#pullrequestreview-2097973059
PR Review Comment: https://git.openjdk.org/jdk/pull/19549#discussion_r1626892779
PR Review Comment: https://git.openjdk.org/jdk/pull/19549#discussion_r1626878077


More information about the hotspot-runtime-dev mailing list