[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: 8240254: Build is broken when cds is disabled after JDK-8236604(Internet mail)
From: jiefu(傅杰) <jiefu () tencent ! com>
Date: 2020-02-29 2:38:36
Message-ID: 36B3838B-285E-48AD-9093-20385265B767 () tencent ! com
[Download RAW message or body]
Hi Yumin and Calvin,
I think it will improve the readabiliy of the code if quick_resolve is guarded.
Thanks.
Best regards,
Jie
From: Yumin Qi <yumin.qi@oracle.com>
Date: Saturday, February 29, 2020 at 10:32 AM
To: Calvin Cheung <calvin.cheung@oracle.com>, "jiefu(傅杰)" <jiefu@tencent.com>, \
Claes Redestad <claes.redestad@oracle.com>, "hotspot-runtime-dev@openjdk.java.net" \
<hotspot-runtime-dev@openjdk.java.net>
Subject: Re: 8240254: Build is broken when cds is disabled after JDK-8236604(Internet \
mail)
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@oracle.com><mailto:yumin.qi@oracle.com>
Date: Saturday, February 29, 2020 at 10:03 AM
To: "jiefu(傅杰)" <jiefu@tencent.com><mailto:jiefu@tencent.com>, Claes Redestad \
<claes.redestad@oracle.com><mailto:claes.redestad@oracle.com>, \
"hotspot-runtime-dev@openjdk.java.net"<mailto:hotspot-runtime-dev@openjdk.java.net> \
<hotspot-runtime-dev@openjdk.java.net><mailto:hotspot-runtime-dev@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@oracle.com><mailto:yumin.qi@oracle.com><mailto:yumin.qi@oracle.com><mailto:yumin.qi@oracle.com>
Date: Saturday, February 29, 2020 at 9:49 AM
To: Claes Redestad <claes.redestad@oracle.com><mailto:claes.redestad@oracle.com><mailto:claes.redestad@oracle.com><mailto:claes.redestad@oracle.com>, \
"jiefu(傅杰)" <jiefu@tencent.com><mailto:jiefu@tencent.com><mailto:jiefu@tencent.com><mailto:jiefu@tencent.com>, \
"hotspot-runtime-dev@openjdk.java.net"<mailto:hotspot-runtime-dev@openjdk.java.net><ma \
ilto:hotspot-runtime-dev@openjdk.java.net><mailto:hotspot-runtime-dev@openjdk.java.net> \
<hotspot-runtime-dev@openjdk.java.net><mailto:hotspot-runtime-dev@openjdk.java.net><ma \
ilto:hotspot-runtime-dev@openjdk.java.net><mailto:hotspot-runtime-dev@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic