RFR: 8240254: Build is broken when cds is disabled after JDK-8236604

[Yumin Qi]

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
>>>