RFR: 8279366: CDS should allow alternative locations for JAR files in classpath

[Ioi Lam]

On Fri, 7 Oct 2022 22:59:09 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> Please review this RFE for allowing alternative locations for jar files in the app classpath during runtime.
> 
> During dump time, the longest common prefix of the app classpath will be stored in the CDS archive header. During runtime, existing checks of the app classpath will be performed first. Upon failure, the longest common prefix of the runtime app classpath will be computed and another check of the app classpath will be performed ignoring the longest common prefix.
> 
> Refer to the [bug report comment](https://bugs.openjdk.org/browse/JDK-8279366?focusedCommentId=14509536&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14509536) for an example.
> 
> Passed tiers 1 - 4 testing (including the new tests).

Changes requested by iklam (Reviewer).

src/hotspot/share/cds/filemap.cpp line 261:

> 259:   set_base_archive_name_size((unsigned int)base_archive_name_size);
> 260:   set_common_app_classpath_offset((unsigned int)common_app_classpath_offset);
> 261:   set_common_app_classpath_size((unsigned int)common_app_classpath_size);

I think there's no need to record the actual `common_app_classpath` string into the `FileMapHeader`. It should be enough to just store the length of the longest common path prefix. E.g., at runtime, the function `FileMapInfo::check_paths_ignoring_common_path` currently only uses `strlen(dumptime_prefix)`.

Also, `longest_common_app_classpath` can just return the length instead of the string.

src/hotspot/share/cds/filemap.cpp line 895:

> 893:   for (int i = 1; i < num_paths; i++) {
> 894:     chr = strrchr((char*)rp_array->at(i), *os::file_separator());
> 895:     size_t curr_len = chr != NULL ? chr - rp_array->at(i) + 1 : strlen(rp_array->at(i)) + 1;

If `chr == NULL`, it means we have a file name only, with no file separator. In this case, there will be no common prefix, so this function can return NULL immediately.

src/hotspot/share/cds/filemap.cpp line 915:

> 913:     *(lcp + j) = c;
> 914:   }
> 915: 

Need to check for this case as well:


rp[0] = "/x/a/Foo.jar"
rp[1] = "/x/aa/Bar.jar"


I think the current code will incorrectly return "/x/a"

src/hotspot/share/cds/filemap.cpp line 942:

> 940:     const char* runtime_path = rp_array->at(i)  + strlen(runtime_prefix);
> 941:     if (strcmp(dumptime_path, runtime_path) != 0) {
> 942:       mismatch = true;

Can this be simplified as `return false` so you don't need to maintain the `mismatch` variable?

src/hotspot/share/cds/filemap.cpp line 1079:

> 1077:         mismatch = check_paths_ignoring_common_path(j, shared_app_paths_len, rp_array,
> 1078:                                                     (const char*)dumptime_prefix, runtime_prefix);
> 1079:       }

For completeness, we should allow empty prefixes. For example:

dump: `-cp foo.jar:x/bar.jar`
run: `-cp a/foo.jar:a/x/bar.jar`

or

dump: `-cp a/foo.jar:a/x/bar.jar`
run: `-cp foo.jar:x/bar.jar`

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

PR: https://git.openjdk.org/jdk/pull/10615