On Sat, 2 Mar 2024 01:18:06 GMT, Ioi Lam <[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()`.
Looks good. Just one nit below.
I assume that it has gone through some testing?
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.
-------------
Marked as reviewed by ccheung (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18095#pullrequestreview-1918091197
PR Review Comment: https://git.openjdk.org/jdk/pull/18095#discussion_r1513475007