RFR: 8319999: Refactor MetaspaceShared::use_full_module_graph() [v5]

Calvin Cheung ccheung at openjdk.org
Tue Nov 14 22:29:34 UTC 2023


On Tue, 14 Nov 2023 15:43:46 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This is another step of moving CDS config management into cdsConfig.hpp:
>> 
>> The function `MetaspaceShared::use_full_module_graph()` is split into two:
>> - `CDSConfig::is_dumping_full_module_graph()`
>> - `CDSConfig::is_loading_full_module_graph()`
>
> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into 8319999-refactor-metaspaceshared-use-full-module-graph
>  - fixed white spaces
>  - Changed flag to CDSConfig::_dumping_full_module_graph_disabled
>  - rename FileMapHeader::_use_full_module_graph -> _has_full_module_graph
>  - 8319999: Refactor MetaspaceShared::use_full_module_graph()

Spotted a few minor items.

src/hotspot/share/cds/filemap.hpp line 283:

> 281:   void set_heap_roots_offset(size_t n)           { _heap_roots_offset = n; }
> 282:   void copy_base_archive_name(const char* name);
> 283: 

Line removed by accident?

src/hotspot/share/classfile/classLoaderDataShared.cpp line 185:

> 183: 
> 184: void ClassLoaderDataShared::clear_archived_oops() {
> 185:   assert(UseSharedSpaces && !CDSConfig::is_loading_full_module_graph(), "must be");

Is the `UseSharedSpaces `needed?

src/hotspot/share/classfile/modules.cpp line 608:

> 606: 
> 607: void Modules::define_archived_modules(Handle h_platform_loader, Handle h_system_loader, TRAPS) {
> 608:   assert(UseSharedSpaces && CDSConfig::is_loading_full_module_graph(), "must be");

Is the `UseSharedSpaces` needed?

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

PR Review: https://git.openjdk.org/jdk/pull/16646#pullrequestreview-1730911156
PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1393372200
PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1393379538
PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1393381654


More information about the hotspot-dev mailing list