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

[Yumin Qi]

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>
> *Date: *Saturday, February 29, 2020 at 9:49 AM
> *To: *Claes Redestad <claes.redestad at oracle.com>, "jiefu(傅杰)" 
> <jiefu at tencent.com>, "hotspot-runtime-dev at openjdk.java.net" 
> <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
>