RFR: 8320935: Move CDS config initialization code to cdsConfig.cpp

[Matias Saavedra Silva]

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

This looks good! I think this is a good opportunity to refactor some of the code for better readability so I left some comments below.

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

> 99: void CDSConfig::extract_shared_archive_paths(const char* archive_path,
> 100:                                          char** base_archive_path,
> 101:                                          char** top_archive_path) {

Could you align these arguments?

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

> 123: }
> 124: 
> 125: void CDSConfig::init_shared_archive_paths() {

Now that I see this there is a lot of indentation thanks to the nested conditionals. I don't have much to offer but is there a cleaner way to format this method? Maybe you can extract the code in `if (archives  == 1)` into its own method for better readability.

src/hotspot/share/runtime/arguments.cpp line 1262:

> 1260:   }
> 1261: 
> 1262:   CDSConfig::check_system_property(key, value);

I see this is only called once, do you expect this method to be used again? It may be unnecessary to extract this code into its own method.

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

PR Review: https://git.openjdk.org/jdk/pull/16868#pullrequestreview-1760626242
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412613074
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412616593
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412610795