[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8320935: Move CDS config initialization code to cdsConfig.cpp
From:       Calvin Cheung <ccheung () openjdk ! org>
Date:       2023-11-29 22:00:07
Message-ID: 7tqgQeAidnvr6kp8hkHZ4QPCV_pFbVvWbafTiWzEEbg=.0e728f7b-0c77-4012-bc3d-6cec099b9e68 () github ! com
[Download RAW message or body]

On Tue, 28 Nov 2023 23:24:53 GMT, Ioi Lam <iklam@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


[prev in list] [next in list] [prev in thread] [next in thread]