8240254: Build is broken when cds is disabled after JDK-8236604(Internet mail)

Sat Feb 29 02:31:50 UTC 2020

HI, Calvin

   I found that the function load_shared_class_misc already inside

#if INCLUDE_CDS

#endif

   I thought it is the new quick_resolve function needs guarded.  
Anyway, the fix works as expected.

   I will check if the new added quick_resolve need guarded.

Thanks

Yumin

On 2/28/20 6:27 PM, Calvin Cheung wrote:
> Hi Jie,
>
> Before seeing your email, I've filed 
> https://bugs.openjdk.java.net/browse/JDK-8240257 per Yumin's request 
> since he was having some problem logging into JBS.
>
> If the issue has been fixed, can you close the above bug with your 
> latest message in this email thread.
>
> thanks,
>
> Calvin
>
> On 2/28/20 6:18 PM, jiefu(傅杰) wrote:
>> Yes. It passed on my computer.
>>
>> Apple clang version 11.0.0 (clang-1100.0.33.16)
>> Target: x86_64-apple-darwin19.0.0
>>
>> Because SystemDictionary::load_shared_class_misc(InstanceKlass* ik, 
>> ClassLoaderData* loader_data, TRAPS) is guarded by #INCLUDE_CDS.
>> So I think it OK.
>>
>>
>>
>> From: Yumin Qi <yumin.qi at oracle.com>
>> Date: Saturday, February 29, 2020 at 10:03 AM
>> To: "jiefu(傅杰)" <jiefu at tencent.com>, Claes Redestad 
>> <claes.redestad at oracle.com>, "hotspot-runtime-dev at openjdk.java.net" 
>> <hotspot-runtime-dev at openjdk.java.net>
>> Subject: Re: 8240254: Build is broken when cds is disabled after 
>> JDK-8236604(Internet mail)
>>
>>
>> OK, I will file bug for it. Did you get your build passed? How come 
>> the two versions not found by compiler?
>>
>> Thanks
>>
>> Yumin
>> On 2/28/20 5:58 PM, jiefu(傅杰) wrote:
>> Hi Yumin,
>>
>> It's too late.
>> Sorry, I missed your comments.
>>
>> I think you can file a new bug to fix it.
>>
>> Thanks a lot.
>> Best regards,
>> Jie
>>
>> From: Yumin Qi <yumin.qi at oracle.com><mailto:yumin.qi at oracle.com>
>> Date: Saturday, February 29, 2020 at 9:49 AM
>> To: Claes Redestad 
>> <claes.redestad at oracle.com><mailto:claes.redestad at oracle.com>, 
>> "jiefu(傅杰)" <jiefu at tencent.com><mailto:jiefu at tencent.com>, 
>> "hotspot-runtime-dev at openjdk.java.net"<mailto:hotspot-runtime-dev at openjdk.java.net> 
>> <hotspot-runtime-dev at openjdk.java.net><mailto:hotspot-runtime-dev at openjdk.java.net> 
>>
>> Subject: Re: RFR: 8240254: Build is broken when cds is disabled after 
>> JDK-8236604(Internet mail)
>>
>>
>> HI, Claes and Jie
>>
>>    Thanks for reporting this and gave a quick fix. Yes, this function 
>> should be guarded by INCLUDE_CDS:
>>
>>    in systemDictionary.hpp:
>>
>>    static void load_shared_class_misc(InstanceKlass* ik, 
>> ClassLoaderData* loader_data, TRAPS) NO_CDS_RETURN;
>>
>>    in systemDictionary.cpp:
>>
>> iff -r 624d51be7acd src/hotspot/share/classfile/systemDictionary.cpp
>> --- a/src/hotspot/share/classfile/systemDictionary.cpp    Fri Feb 28 
>> 12:49:37 2020 -0800
>> +++ b/src/hotspot/share/classfile/systemDictionary.cpp    Fri Feb 28 
>> 17:46:22 2020 -0800
>> @@ -1918,6 +1918,7 @@
>>   }
>>   #endif
>>
>> +#if INCLUDE_CDS
>>   void SystemDictionary::quick_resolve(InstanceKlass* klass, 
>> ClassLoaderData* loader_data, Handle domain, TRAPS) {
>>     assert(!Universe::is_fully_initialized(), "We can make short cuts 
>> only during VM initialization");
>>     assert(klass->is_shared(), "Must be shared class");
>> @@ -1948,6 +1949,7 @@
>>     add_to_hierarchy(klass, CHECK);
>>     assert(klass->is_loaded(), "Must be in at least loaded state");
>>   }
>> +#endif
>>
>>   bool SystemDictionary::resolve_wk_klass(WKID id, TRAPS) {
>>     assert(id >= (int)FIRST_WKID && id < (int)WKID_LIMIT, "oob");
>>
>>
>>
>> The function needs guarded too or in Not CDS build, it will have two 
>> versions.
>>
>>
>>
>> Thanks
>>
>> Yumin
>>
>>
>>
>>
>> On 2/28/20 5:24 PM, Claes Redestad wrote:
>> Oops, wrong method:
>>
>> diff -r f227e770495f src/hotspot/share/classfile/systemDictionary.hpp
>> --- a/src/hotspot/share/classfile/systemDictionary.hpp    Fri Feb 28 
>> 15:30:29 2020 -0800
>> +++ b/src/hotspot/share/classfile/systemDictionary.hpp    Sat Feb 29 
>> 02:24:12 2020 +0100
>> @@ -600,7 +600,7 @@
>>                                             const ClassFileStream *cfs,
>>                                             TRAPS);
>>     // Second part of load_shared_class
>> -  static void load_shared_class_misc(InstanceKlass* ik, 
>> ClassLoaderData* loader_data, TRAPS);
>> +  static void load_shared_class_misc(InstanceKlass* ik, 
>> ClassLoaderData* loader_data, TRAPS) NO_CDS_RETURN;
>>     static InstanceKlass* load_shared_boot_class(Symbol* class_name,
>>                                                  TRAPS);
>>     static InstanceKlass* load_instance_class(Symbol* class_name, 
>> Handle class_loader, TRAPS);
>>
>>
>> On 2020-02-29 02:21, Claes Redestad wrote:
>>
>>
>> Hi,
>>
>> I think we typically add NOT_CDS_RETURN to the declaration of the method
>> in systemDictionary.hpp:
>>
>> diff -r f227e770495f 
>> src/hotspot/share/classfile/systemDictionaryShared.hpp
>> --- a/src/hotspot/share/classfile/systemDictionaryShared.hpp Fri Feb 
>> 28 15:30:29 2020 -0800
>> +++ b/src/hotspot/share/classfile/systemDictionaryShared.hpp Sat Feb 
>> 29 02:20:09 2020 +0100
>> @@ -271,7 +271,7 @@
>>      }
>>
>>      static void update_shared_entry(InstanceKlass* klass, int id);
>> -  static void set_shared_class_misc_info(InstanceKlass* k, 
>> ClassFileStream* cfs);
>> +  static void set_shared_class_misc_info(InstanceKlass* k, 
>> ClassFileStream* cfs) NO_CDS_RETURN;
>>
>>      static InstanceKlass* lookup_from_stream(Symbol* class_name,
>>                                               Handle class_loader,
>>
>> This should fix your issue with less clutter.
>>
>> Looks good and trivial either way to me.
>>
>> Thanks!
>>
>> /Claes
>>
>> On 2020-02-29 02:06, jiefu(傅杰) wrote:
>>
>>
>> Hi all,
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240254
>>
>> Build is broken when cds is disabled.
>>
>> It might be fixed by
>> ------------------------------------
>> diff -r f227e770495f src/hotspot/share/classfile/systemDictionary.cpp
>> --- a/src/hotspot/share/classfile/systemDictionary.cpp  Fri Feb 28 
>> 15:30:29 2020 -0800
>> +++ b/src/hotspot/share/classfile/systemDictionary.cpp  Sat Feb 29 
>> 08:42:41 2020 +0800
>> @@ -1941,7 +1941,9 @@
>>      }
>>
>>      klass->restore_unshareable_info(loader_data, domain, THREAD);
>> +#if INCLUDE_CDS
>>      load_shared_class_misc(klass, loader_data, CHECK);
>> +#endif
>>      Dictionary* dictionary = loader_data->dictionary();
>>      unsigned int hash = dictionary->compute_hash(klass->name());
>>      dictionary->add_klass(hash, klass->name(), klass);
>> ------------------------------------
>>
>> Could you please review it and give me some advice?
>>
>> Thanks a lot.
>> Best regards,
>> Jie