On Tue, 5 Mar 2024 20:39:10 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> A few clean ups: >> >> 1. Rename functions like "`s_loading_full_module_graph()` to >> `is_using_full_module_graph()`. The meaning of "loading" is not clear: it >> might be interpreted as to cover only the period where the artifact is being >> loaded, but not the period after the artifact is completely loaded. However, >> the function is meant to cover both periods, so "using" is a more precise >> term. >> >> 2. The cumbersome sounding `disable_loading_full_module_graph()` is changed >> to `stop_using_full_module_graph()`, etc. >> >> 3. The status of `is_using_optimized_module_handling()` is moved from >> metaspaceShared.hpp to cdsConfig.hpp, to be consolidated with other types of >> CDS status. >> >> 4. The status of CDS was communicated to the Java class >> `jdk.internal.misc.CDS` by ad-hoc native methods. This is now changed to a >> single method, `CDS.getCDSConfigStatus()` that returns a bit field. That way >> we don't need to add a new native method for each type of status. >> >> 5. `CDS.isDumpingClassList()` was a misnomer. It's changed to >> `CDS.isLoggingLambdaFormInvokers()`. > > 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). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18095#discussion_r1513626841