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