RFR (L) 8194812: Extend class-data sharing to support the module path

[Jiangli Zhou]

Hi Calvin,

The changes for supporting module path for CDS/AppCDS looks very good now. I have just few comments below.

filemap.cpp

- The following check should be applied for the module path as well. We can’t support directory in module path. I think ‘&& is_class_path’ should be removed.
 242   } else if (is_dir() && is_class_path) {
 243     if (!os::dir_is_empty(name)) {
 244       FileMapInfo::fail_continue("directory is not empty: %s", name);
 245       ok = false;
 246     }
Also, could you please add comments for the special handling for module path entry at line 239. If the archived module path entry does not exist at runtime, it is not fatal (no need to invalid the shared archive) because the shared runtime visibility check filters out any archived module classes that do not have a matching runtime module path location.

- Please add assert for UseSharedSpaces in validate() and validate_shared_path_table().

classLoader.cpp

- The following change looks new in the latest webrev. These are for the NULL class loader case, right? Can you please assert the loader indeed is the NULL class loader.
1627               if ((i >= 1) &&
1628                   (i < ClassLoaderExt::app_module_paths_start_index())) {
1629                 classpath_index = i;
1630                 break;
1631               }
- The following comment could be misleading and needs a little rework. Named modules can from the runtime image. The code below the comment handles the module path case only. Please clarify that in the comment.
1634             // non-NULL pkg_entry and is a named module implies the class is from the
1635             // --module-path. Ensure the index is within the --module-path range before
1636             // assigning to the classpath_index.

Minor nits:

classLoader.cpp

- Please fix the indentation at line 1100.

- The variable ‘e’ is not used. Please remove.
1102   ClassPathEntry* e = _module_path_entries;
- Please add comments for the ‘else’ case at line 1661-1665. The shared path table is set up after module system initialization. The path table contains no entry before that. Any classes loaded priorly must be from the runtime image.

- Move the block of code at 1098 ~ 1131 to line 730, so these functions are close to the code that actually uses them. And we can also have one less #if INCLUDE_CDS.

- Please assert DumpSharedSpaces for these dump-time-only functions in add_tomodule_path_entries() and update_module_path_entry_list().

Thanks,
Jiangli

> On Apr 2, 2018, at 2:30 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8194812
> 
> webrev: http://cr.openjdk.java.net/~ccheung/8194812/webrev.00/
> 
> Design write-up: http://cr.openjdk.java.net/~ccheung/8194812/write-up/module-path-design.pdf
> 
> Testing: hs-tier1 through hs-tier4 (in progress, no new failures so far).
> 
> thanks,
> Calvin