RFR 8197954 Remove unnecessary intermediary APIs from AppCDS implementation

[Jiangli Zhou]

Hi Ioi,

Thank you for removing the _ext files and merging the code together. It’s very helpful. 

- sharedPathsMiscInfo.cpp

 191     // FIXME: why is there no MODULE check?

The path check for archived module classes is handled differently than the classes from the -cp or -Xbootclasspath/a because we can quickly determine if an archived module class’ origin matches with the runtime module source path. That’s why we don’t  do string comparison of the dump time module path and the runtime module path in SharedPathsMiscInfo::check(), which is too restrictive and limited. Could you please replace the FIXME comment?

- sharedPathsMiscInfo.hpp

 131   const char* type_name(int type) {
 132     switch (type) {
 133     case BOOT:      return "BOOT";
 134     case NON_EXIST: return "NON_EXIST";
 <> 135     case APP:       return "APP";
 136     case MODULE:    return "MODULE";

The types are not well-defined. That’s not introduced by your change. The ‘APP’ type became inaccurate when we added the ‘MODULE’ type. It's noticeable now when types are merged together. To avoid overlapping, ‘APP' can be changed to ‘CLASS_PATH’ or ‘CP’. How about renaming the types to following:

BOOT_PATH
CLASS_PATH
MODULE_PATH
NON_EXIST

As your current change does not involve larger scale restructuring, it’s okay if we follow up with a separate RFE for the type renaming. 

- systemDictionaryShared.cpp

 490 // [c] At this point, if the named class was loaded by the
 491 //     AppClassLoader during archive dump time, we know that it must be
 492 //     loaded by the AppClassLoader during run time, and will not be loaded
 493 //     by a delegated class loader. 
The above only applies to the application classes loaded from -cp, but not modules. An archived module class may not be used at runtime if the module has a different source path at runtime. Those archived module classes are filtered out by the runtime shared class ‘visibility’ check.

 493 //     by a delegated class loader. This is true because we have checked the
 494 //     CLASSPATH and module path to ensure compatibility between dump time and
 495 //     run time.

Please remove ‘module path’ from above comment. We don’t do string comparison of the runtime module path and dump time module path as CLASSPATH.

The comment [C] probably should be split into two part, one for application classes from -cp and the other part for application module classes.

508 // An alternative is to modify the Java code of BuiltinClassLoader.loadClassOrNull().
Should above be removed? Otherwise, more details are needed.

Thanks,
Jiangli

> On May 1, 2018, at 10:17 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> https://bugs.openjdk.java.net/browse/JDK-8197954
> http://cr.openjdk.java.net/~iklam/jdk11/8197954-remove-unnecessary-appcds-api.v01/
> 
> Summary:
> 
> Before AppCDS was open sourced, we had a few "Ext" classes in sharedClassUtil.hpp
> that abstracted away the CDS operations related to the non-boot class loaders.
> This API made it possible to build the Oracle JDK with AppCDS included, or build
> the Open JDK with AppCDS removed.
> 
> With the open sourcing of AppCDS, this abstraction layer is no longer necessary. I
> have moved the contents of the "Ext" classes into their proper locations and removed
> the sharedClassUtil.hpp/cpp files.
> 
> Most of the changes are just moving things around. There shouldn't be behavioral
> changes.
> 
> The files classLoaderExt.hpp/cpp still exists -- it encapsulates the classpath
> management code and is not strictly unnecessary. Some clean up may be needed there, but
> I'll probably do that in a separate RFE.
> 
> Testing with hotspot tiers1~4.
> 
> Thanks
> - Ioi
> 
>