RFR: 8291840: Avoid JavaCalls for setting up _java_system_loader and _java_platform_loader [v2]

[Calvin Cheung]

On Wed, 3 Aug 2022 23:01:42 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @iklam comments
>
> src/hotspot/share/classfile/modules.cpp line 506:
> 
>> 504: 
>> 505:   ClassLoaderData* system_loader_data = SystemDictionary::register_loader(h_system_loader);
>> 506:   SystemDictionary::set_system_loader(system_loader_data);
> 
> I think we should add an assert here:
> 
> 
> // system_loader_data here is always an instance of jdk.internal.loader.ClassLoader$AppClassLoader.
> // However, if -Djava.system.class.loader=xxx is specified, java_platform_loader() would
> // be an instance of a user-defined class, so make sure this never happens.
> assert(Arguments::get_property("java.system.class.loader") == NULL,
>          "archived full module should have been disabled");

Done.

> src/hotspot/share/classfile/systemDictionary.cpp line 134:
> 
>> 132: 
>> 133:     _java_system_loader = OopHandle(Universe::vm_global(), result.get_oop());
>> 134:   }
> 
> To check that the we correctly restored _java_system_loader from CDS, I think we should move the Java upcall to a worker function and add some asserts:
> 
> 
> if (_java_system_loader.is_empty()) {
>     _java_system_loader = get_system_classloader_impl();
> } else {
>    // It must have been restored from the archived module graph
>    assert(UseSharedSpaces, "must be");
>    assert(MetaspaceShared::use_full_module_graph(). "must be");
>    assert(_java_system_loader.resolve() == get_system_classloader_impl(), "must be");
> }

Done.

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

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