8223171: Redundant nmethod dependencies for effectively final methods
authorvlivanov
Mon, 06 May 2019 12:15:49 -0700
changeset 54721 3661ad97da8f
parent 54720 c48f141e7c5b
child 54722 f0bce2f93e72
8223171: Redundant nmethod dependencies for effectively final methods Reviewed-by: dlong
src/hotspot/share/c1/c1_GraphBuilder.cpp
src/hotspot/share/ci/ciMethod.cpp
src/hotspot/share/ci/ciMethod.hpp
src/hotspot/share/code/dependencies.cpp
src/hotspot/share/code/dependencies.hpp
src/hotspot/share/oops/method.cpp
src/hotspot/share/oops/method.hpp
src/hotspot/share/opto/doCall.cpp
--- 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;
   }