On Sat, 2 Mar 2024 01:18:06 GMT, Ioi Lam <ik...@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()`.

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

Reply via email to