RFR:8187118, 8187119: Removing -cp class path entries from boot append list at CDS dump time and other cleanups

[Ioi Lam]

Looks good. Thanks!

- Ioi


On 11/17/17 4:54 PM, Lois Foltan wrote:
> Looks good!
> Lois
>
> On 11/17/2017 7:05 PM, Jiangli Zhou wrote:
>> Please review the changes for following two RFEs.
>>
>> 8187118: Remove appending -cp path to the boot class path at AppCDS 
>> dump time. 
>> https://bugs.openjdk.java.net/browse/JDK-8187118?filter=14921 
>> <https://bugs.openjdk.java.net/browse/JDK-8187118?filter=14921>
>> 8187119: Consolidate record_shared_class_loader_type() and 
>> record_result(). 
>> https://bugs.openjdk.java.net/browse/JDK-8187119?filter=14921 
>> <https://bugs.openjdk.java.net/browse/JDK-8187119?filter=14921>
>>
>> webrev: http://cr.openjdk.java.net/~jiangli/8187118_8187119/webrev.01 
>> <http://cr.openjdk.java.net/~jiangli/8187118_8187119/webrev.01>
>>
>> With the changes in above webrev, I’ve done some cleanups in 
>> CDS/AppCDS code:
>>
>> - Removed -cp class path entries from boot append list at CDS dump 
>> time as the application classes are no longer loaded by the boot 
>> loader at CDS dump time. The -cp path entries are maintained in a 
>> separate list by ClassLoader now. CDS code creates the shared class 
>> path entry table by combining the boot append list an -cp path entry 
>> list (see FileMapInfo::allocate_classpath_entry_table()). Split 
>> ClassLoaderExt::add_class_path_entry() logic into 
>> add_to_boot_append_entries() (renamed from 
>> ClassLoader::add_to_list()) and add_to_classpath_entries().
>>
>> - Cleaned up SystemDictionary::load_instance_class() to remove the 
>> handling of DumpSharedSpaces cases from general class loading code.
>>
>> - Separated ClassLoader::add_package() call from 
>> ClassLoaderExt::Context::record_result(). Calling add_package() in 
>> record_result() complicates things unnecessarily. 
>> ClassLoader::add_package() is needed for classes loaded by the boot 
>> loader, while Context::record_result() is only needed for CDS dump 
>> time. Logically they are unrelated. Separating the two makes the code 
>> much cleaner.
>>
>> - Renamed ClassLoader::record_shared_class_loader_type() to 
>> ClassLoader::record_result(). Cleaned up the usage of 
>> ClassLoader::record_shared_class_loader_type() and 
>> Context::record_result(). Before the clean up, 
>> Context::record_result() was called twice during loading of classes 
>> for the boot loader at CDS dump time, and the classes for boot loader 
>> and PlatformClassLoader/AppClassLoader were handled differently. Now 
>> all classes loaded by the builtin class loaders at CDS dump time are 
>> handled the same.
>>
>> - Removed the use of ClassLoader::_num_entries and _num_boot_entries.
>> - Renamed ClassLoader::setup_search_path() to setup_boot_search_path().
>> - Removed unused ClassLoader::add_to_list(const char*)
>>
>> Tested with hotspot/runtime locally on linux-x64. Ran tier1, tier2, 
>> tier3, teir4 and tier5 tests.
>>
>> Thanks,
>> Jiangli
>