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

[Ioi Lam]

On Fri, 1 Dec 2023 22:04:22 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed indentation
>
> 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?

Fixed.

> 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.

I want to keep the code change minimal while moving code from one file to another. I'll refactor this function in a follow-on PR. That way it will be easier to track the code history.

> 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.

I wanted to move the code from arguments.cpp to cdsConfig.cpp, so I had to put it in a new function.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412683767
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412683760
PR Review Comment: https://git.openjdk.org/jdk/pull/16868#discussion_r1412683786