8223171: Redundant nmethod dependencies for effectively final methods
Reviewed-by: dlong
--- a/src/hotspot/share/c1/c1_GraphBuilder.cpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp Mon May 06 12:15:49 2019 -0700
@@ -1988,8 +1988,9 @@
}
if (cha_monomorphic_target != NULL) {
+ assert(!target->can_be_statically_bound() || target == cha_monomorphic_target, "");
assert(!cha_monomorphic_target->is_abstract(), "");
- if (!target->is_final_method() && !target->is_private()) {
+ if (!cha_monomorphic_target->can_be_statically_bound(actual_recv)) {
// If we inlined because CHA revealed only a single target method,
// then we are dependent on that target method not getting overridden
// by dynamic class loading. Be sure to test the "static" receiver
--- a/src/hotspot/share/ci/ciMethod.cpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/ci/ciMethod.cpp Mon May 06 12:15:49 2019 -0700
@@ -765,6 +765,14 @@
}
// ------------------------------------------------------------------
+// ciMethod::can_be_statically_bound
+//
+// Tries to determine whether a method can be statically bound in some context.
+bool ciMethod::can_be_statically_bound(ciInstanceKlass* context) const {
+ return (holder() == context) && can_be_statically_bound();
+}
+
+// ------------------------------------------------------------------
// ciMethod::resolve_invoke
//
// Given a known receiver klass, find the target for the call.
--- a/src/hotspot/share/ci/ciMethod.hpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/ci/ciMethod.hpp Mon May 06 12:15:49 2019 -0700
@@ -352,6 +352,8 @@
bool is_unboxing_method() const;
bool is_object_initializer() const;
+ bool can_be_statically_bound(ciInstanceKlass* context) const;
+
// Replay data methods
void dump_name_as_ascii(outputStream* st);
void dump_replay_data(outputStream* st);
--- a/src/hotspot/share/code/dependencies.cpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/code/dependencies.cpp Mon May 06 12:15:49 2019 -0700
@@ -108,6 +108,7 @@
void Dependencies::assert_unique_concrete_method(ciKlass* ctxk, ciMethod* uniqm) {
check_ctxk(ctxk);
+ check_unique_method(ctxk, uniqm);
assert_common_2(unique_concrete_method, ctxk, uniqm);
}
@@ -180,6 +181,7 @@
void Dependencies::assert_unique_concrete_method(Klass* ctxk, Method* uniqm) {
check_ctxk(ctxk);
+ check_unique_method(ctxk, uniqm);
assert_common_2(unique_concrete_method, DepValue(_oop_recorder, ctxk), DepValue(_oop_recorder, uniqm));
}
--- a/src/hotspot/share/code/dependencies.hpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/code/dependencies.hpp Mon May 06 12:15:49 2019 -0700
@@ -27,6 +27,7 @@
#include "ci/ciCallSite.hpp"
#include "ci/ciKlass.hpp"
+#include "ci/ciMethod.hpp"
#include "ci/ciMethodHandle.hpp"
#include "classfile/systemDictionary.hpp"
#include "code/compressedStream.hpp"
@@ -341,6 +342,9 @@
check_ctxk(ctxk);
assert(!is_concrete_klass(ctxk->as_instance_klass()), "must be abstract");
}
+ static void check_unique_method(ciKlass* ctxk, ciMethod* m) {
+ assert(!m->can_be_statically_bound(ctxk->as_instance_klass()), "redundant");
+ }
void assert_common_1(DepType dept, ciBaseObject* x);
void assert_common_2(DepType dept, ciBaseObject* x0, ciBaseObject* x1);
@@ -368,6 +372,10 @@
check_ctxk(ctxk);
assert(ctxk->is_abstract(), "must be abstract");
}
+ static void check_unique_method(Klass* ctxk, Method* m) {
+ assert(!m->can_be_statically_bound(InstanceKlass::cast(ctxk)), "redundant");
+ }
+
void assert_common_1(DepType dept, DepValue x);
void assert_common_2(DepType dept, DepValue x0, DepValue x1);
--- a/src/hotspot/share/oops/method.cpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/oops/method.cpp Mon May 06 12:15:49 2019 -0700
@@ -624,6 +624,10 @@
return can_be_statically_bound(method_holder()->access_flags());
}
+bool Method::can_be_statically_bound(InstanceKlass* context) const {
+ return (method_holder() == context) && can_be_statically_bound();
+}
+
bool Method::is_accessor() const {
return is_getter() || is_setter();
}
--- a/src/hotspot/share/oops/method.hpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/oops/method.hpp Mon May 06 12:15:49 2019 -0700
@@ -619,6 +619,7 @@
// true if method needs no dynamic dispatch (final and/or no vtable entry)
bool can_be_statically_bound() const;
+ bool can_be_statically_bound(InstanceKlass* context) const;
bool can_be_statically_bound(AccessFlags class_access_flags) const;
// returns true if the method has any backward branches.
--- a/src/hotspot/share/opto/doCall.cpp Mon May 06 14:47:55 2019 -0400
+++ b/src/hotspot/share/opto/doCall.cpp Mon May 06 12:15:49 2019 -0700
@@ -1152,14 +1152,19 @@
cha_monomorphic_target = NULL;
}
}
+
if (cha_monomorphic_target != NULL) {
// Hardwiring a virtual.
- // If we inlined because CHA revealed only a single target method,
- // then we are dependent on that target method not getting overridden
- // by dynamic class loading. Be sure to test the "static" receiver
- // dest_method here, as opposed to the actual receiver, which may
- // falsely lead us to believe that the receiver is final or private.
- dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
+ assert(!callee->can_be_statically_bound(), "should have been handled earlier");
+ assert(!cha_monomorphic_target->is_abstract(), "");
+ if (!cha_monomorphic_target->can_be_statically_bound(actual_receiver)) {
+ // If we inlined because CHA revealed only a single target method,
+ // then we are dependent on that target method not getting overridden
+ // by dynamic class loading. Be sure to test the "static" receiver
+ // dest_method here, as opposed to the actual receiver, which may
+ // falsely lead us to believe that the receiver is final or private.
+ dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
+ }
return cha_monomorphic_target;
}