RFR: 8327138: Clean up status management in cdsConfig.hpp and CDS.java

Calvin Cheung ccheung at openjdk.org
Tue Mar 5 23:51:45 UTC 2024


On Tue, 5 Mar 2024 23:38:21 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/cds/cdsConfig.hpp line 94:
>> 
>>> 92:   static bool   is_dumping_full_module_graph()               { return CDS_ONLY(_is_dumping_full_module_graph) NOT_CDS(false); }
>>> 93:   static void   stop_using_full_module_graph(const char* reason = nullptr) NOT_CDS_JAVA_HEAP_RETURN;
>>> 94:   static bool     is_using_full_module_graph()                NOT_CDS_JAVA_HEAP_RETURN_(false);
>> 
>> Please align this with line #90.
>
> Do you mean aligning like this: 
> 
> 
>   static bool is_dumping_heap()                              NOT_CDS_JAVA_HEAP_RETURN_(false);
>   static void stop_dumping_full_module_graph(const char* reason = nullptr) NOT_CDS_JAVA_HEAP_RETURN;
>   static bool is_dumping_full_module_graph()                 { return CDS_ONLY(_is_dumping_full_module_graph) NOT_CDS(false); }
>   static void stop_using_full_module_graph(const char* reason = nullptr) NOT_CDS_JAVA_HEAP_RETURN;
>   static bool is_using_full_module_graph()                   NOT_CDS_JAVA_HEAP_RETURN_(false);
> 
> 
> I think this is more messy, and you can't easily tell that these four functions are all related to the same feature (full_module_graph).

I meant the following. Just the last line #94 needs to be changed - shift one space to the left after `bool`.

static bool   is_dumping_heap()                            NOT_CDS_JAVA_HEAP_RETURN_(false);
static void stop_dumping_full_module_graph(const char* reason = nullptr) NOT_CDS_JAVA_HEAP_RETURN;
static bool   is_dumping_full_module_graph()               { return CDS_ONLY(_is_dumping_full_module_graph) NOT_CDS(false); }
static void   stop_using_full_module_graph(const char* reason = nullptr) NOT_CDS_JAVA_HEAP_RETURN;
static bool   is_using_full_module_graph()                 NOT_CDS_JAVA_HEAP_RETURN_(false);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18095#discussion_r1513633931


More information about the core-libs-dev mailing list