On Tue, 28 Nov 2023 23:24:53 GMT, Ioi Lam <ik...@openjdk.org> wrote:

> This is a simple clean up that moves the code for initializing the CDS config 
> states from arguments.cpp to cdsConfig.cpp
> 
> I renamed a few functions, but otherwise the code is unchanged.
> 
> - `get_default_shared_archive_path()` -> `default_archive_path()`
> - `GetSharedArchivePath()` -> `static_archive_path()`
> - `GetSharedDynamicArchivePath()` -> `dynamic_archive_path()`
> 
> There's also less `#if INCLUDE_CDS` since the entire cdsConfig.cpp file is 
> compiled only if CDS is enabled.

Code migration from arguments cpp to cdsConfig.cpp looks good.
Found a minor simplification regarding the include statements.

src/hotspot/share/cds/cdsConfig.cpp line 34:

> 32: #include "logging/log.hpp"
> 33: #include "runtime/arguments.hpp"
> 34: #include "runtime/java.hpp"

I was able to build with your patch without including `java.hpp`.
The #include java.hpp could also be removed from arguments.cpp.

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

PR Review: https://git.openjdk.org/jdk/pull/16868#pullrequestreview-1756281866
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1409910394

Reply via email to