On Tue, 5 Mar 2024 20:39:10 GMT, Calvin Cheung <[email protected]> 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
