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

[jiefu(傅杰)]

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