RFR(S): 8193434: [GRAAL] Graal classes are not loaded with -Xshare:dump

[Calvin Cheung]

Hi Ioi,

Thanks for taking a look.

On 2/27/18, 4:30 PM, Ioi Lam wrote:
> Hi Calvin,
>
> I think the CNFE is caused by this code (in the "before" version):
>
> 1054 InstanceKlass* SystemDictionary::resolve_from_stream(Symbol* 
> class_name,
> 1055 Handle class_loader,
> 1056 Handle protection_domain,
> 1057 ClassFileStream* st,
> 1058 TRAPS) {
> 1059 #if INCLUDE_CDS
> 1060   ResourceMark rm(THREAD);
> 1061   if (DumpSharedSpaces && !class_loader.is_null() &&
> 1062       !UseAppCDS && strcmp(class_name->as_C_string(), "Unnamed") 
> != 0) {
> 1063     // If AppCDS is not enabled, don't define the class at dump 
> time (except for the "Unnamed"
> 1064     // class, which is used by MethodHandles).
> 1065 THROW_MSG_NULL(vmSymbols::java_lang_ClassNotFoundException(), 
> class_name->as_C_string());
> 1066   }
> 1067 #endif
Yes, I wrote a comment in the bug report yesterday.
>
> As far as I can tell, the intention is -- if UseAppCDS is not enabled, 
> load classes only for the boot loader.
Right.
>
> However, when -XX:+UseJVMCICompiler is enabled in the command-line 
> during dumping, we get this:
>
> $ java -Xshare:dump -Xlog:cds,cds+hashtables 
> -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=./test.jsa 
> -Xbatch -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI 
> -XX:+UseJVMCICompiler
>
> java.lang.ClassNotFoundException: 
> org/graalvm/compiler/hotspot/HotSpotGraalJVMCIServiceLocator
>     at java.lang.ClassLoader.defineClass2(java.base at 11-internal/Native 
> Method)
>     at 
> java.lang.ClassLoader.defineClass(java.base at 11-internal/ClassLoader.java:1086)
>     at 
> java.security.SecureClassLoader.defineClass(java.base at 11-internal/SecureClassLoader.java:206)
>     at 
> jdk.internal.loader.BuiltinClassLoader.defineClass(java.base at 11-internal/BuiltinClassLoader.java:760)
>     at 
> jdk.internal.loader.BuiltinClassLoader.findClassInModuleOrNull(java.base at 11-internal/BuiltinClassLoader.java:681)
>     at 
> jdk.internal.loader.BuiltinClassLoader.findClass(java.base at 11-internal/BuiltinClassLoader.java:562)
>     at 
> java.lang.ClassLoader.loadClass(java.base at 11-internal/ClassLoader.java:611)
>     at java.lang.Class.forName(java.base at 11-internal/Class.java:450)
>     ...
>
>
> With "-XX+UseAppCDS -Xlog:class+load=debug", we can see this class is 
> loaded by the platform loader:
>
>
> [0.325s][info ][class,load] 
> org.graalvm.compiler.hotspot.HotSpotGraalJVMCIServiceLocator source: 
> jrt:/jdk.internal.vm.compiler
> [0.325s][debug][class,load]  klass: 0x00000008c0083c30 super: 
> 0x00000008c00734c8 loader: [class loader 0x00007f30344e5870 a 
> 'jdk/internal/loader/ClassLoaders$PlatformClassLoader'{0x000000041105cbf0}] 
> bytes: 1443 checksum: 8dae8ade
>
>
> I think instead of using the _force_init_jvmci_runtime, which allows 
> these classes to be loaded only during a portion of the dump time, a 
> better fix would be to remove the above "if" block altogether. I think 
> if UseAppCDS is not true, 
> SystemDictionary::combine_shared_dictionaries() won't be called, so 
> any non-boot classes will be left out from the CDS archive, which is 
> what you want.
I tried setting breakpoint in 
SystemDictionary::combine_shared_dictionaries() and the debugger stopped 
there without enabling UseAppCDS. My current proposed change should have 
the least impact on CDS (without enabling UseAppCDS).
For getting rid of those "Preload Warning" noted by Vladimir, I think I 
can leave the _force_init_jvmci_runtime setting; i.e. not resetting it 
to false at the end of force JVMCI runtime init. This is what I had in 
my first version:
http://cr.openjdk.java.net/~ccheung/8193434/webrev.00/
>
> What do you think?
I think the current fix with the "Preload Warning" is ok; it means 170 
classes won't be in the archive.

thanks,
Calvin
>
> Thanks
> - Ioi
>
>
>
> On 2/27/18 11:01 AM, Calvin Cheung wrote:
>> bug: https://bugs.openjdk.java.net/browse/JDK-8193434
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8193434/webrev.01/
>>
>> java -Xshare:dump -Xlog:cds,cds+hashtables 
>> -XX:+UnlockDiagnosticVMOptions -XX:SharedArchiveFile=./test.jsa 
>> -Xbatch -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI 
>> -XX:+UseJVMCICompiler -Djvmci.Compiler=graal
>>
>> The above command causes the CNFE to be thrown in 
>> SystemDictionary::resolve_from_stream() during CDS dump time.
>> The proposed change is to add a _force_init_jvmci_runtime flag which 
>> is set at the beginning of JVMCIRuntime::force_initialization() and 
>> reset at the end of the initialization. The flag will be checked in 
>> SystemDictionary::resolve_from_stream() so that it will not throw 
>> CNFE during the eager initialization of JVMCI runtime during vm init.
>>
>> The proposed change passed hs-tier1 through hs-tier3 testing on 
>> linux-x64, windows-x64, sparcv9, and Mac OS X.
>>
>> thanks,
>> Calvin
>>
>