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

Ioi Lam iklam at openjdk.org
Tue Nov 14 04:59:31 UTC 2023


On Tue, 14 Nov 2023 04:30:37 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   rename FileMapHeader::_use_full_module_graph -> _has_full_module_graph
>
> src/hotspot/share/cds/cdsConfig.hpp line 33:
> 
>> 31: class CDSConfig : public AllStatic {
>> 32: #if INCLUDE_CDS
>> 33:   static bool     _is_dumping_dynamic_archive;
> 
> Nit: Extra spaces

The alignment is the same as the is_dumping_xxx()/enable_dumping_xxx() functions below, so it's easier to read the "dumping" and "loading" part of the names.

> src/hotspot/share/cds/filemap.cpp line 216:
> 
>> 214:   _max_heap_size = MaxHeapSize;
>> 215:   _use_optimized_module_handling = MetaspaceShared::use_optimized_module_handling();
>> 216:   _has_full_module_graph = CDSConfig::is_dumping_full_module_graph();
> 
> `_uses_full_module_graph` would seem more appropriate than `use` or `has`

The name is consistent with other fields in filemap.hpp:


  bool _has_non_jar_in_classpath;     // non-jar file entry exists in classpath
  bool _has_platform_or_app_classes;  // Archive contains app classes
  bool _has_full_module_graph;        // Does this CDS archive contain the full archived module graph?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1391984645
PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1391983234


More information about the hotspot-dev mailing list