8024368: private methods are allocated vtable slots
authorsspitsyn
Wed, 24 Oct 2018 13:11:54 -0700
changeset 52276 ad9077f044be
parent 52275 f39073b97db7
child 52277 c2f38eb6b31a
8024368: private methods are allocated vtable slots Summary: Stop allocating vtable slots for private methods Reviewed-by: dholmes, acorn, lfoltan
src/hotspot/share/oops/klassVtable.cpp
--- a/src/hotspot/share/oops/klassVtable.cpp	Wed Oct 24 09:56:03 2018 -0700
+++ b/src/hotspot/share/oops/klassVtable.cpp	Wed Oct 24 13:11:54 2018 -0700
@@ -84,6 +84,7 @@
     methodHandle mh(THREAD, methods->at(i));
 
     if (needs_new_vtable_entry(mh, super, classloader, classname, class_flags, major_version, THREAD)) {
+      assert(!methods->at(i)->is_private(), "private methods should not need a vtable entry");
       vtable_length += vtableEntry::size(); // we need a new entry
     }
   }
@@ -283,7 +284,7 @@
 // If none found, return a null superk, else return the superk of the method this does override
 // For public and protected methods: if they override a superclass, they will
 // also be overridden themselves appropriately.
-// Private methods do not override and are not overridden.
+// Private methods do not override, and are not overridden and are not in the vtable.
 // Package Private methods are trickier:
 // e.g. P1.A, pub m
 // P2.B extends A, package private m
@@ -390,8 +391,9 @@
     target_method()->set_vtable_index(Method::nonvirtual_vtable_index);
   }
 
-  // Static and <init> methods are never in
-  if (target_method()->is_static() || target_method()->name() ==  vmSymbols::object_initializer_name()) {
+  // Private, static and <init> methods are never in
+  if (target_method()->is_private() || target_method()->is_static() ||
+      (target_method()->name()->fast_compare(vmSymbols::object_initializer_name()) == 0)) {
     return false;
   }
 
@@ -411,10 +413,7 @@
     // valid itable index, if so, don't change it.
     // Overpass methods in an interface will be assigned an itable index later
     // by an inheriting class.
-    // Private interface methods have no itable index and are always invoked nonvirtually,
-    // so they retain their nonvirtual_vtable_index value, and therefore can_be_statically_bound()
-    // will return true.
-    if ((!is_default || !target_method()->has_itable_index()) && !target_method()->is_private()) {
+    if ((!is_default || !target_method()->has_itable_index())) {
       target_method()->set_vtable_index(Method::pending_itable_index);
     }
   }
@@ -425,14 +424,6 @@
     return allocate_new;
   }
 
-  // private methods in classes always have a new entry in the vtable
-  // specification interpretation since classic has
-  // private methods not overriding
-  // JDK8 adds private methods in interfaces which require invokespecial
-  if (target_method()->is_private()) {
-    return allocate_new;
-  }
-
   // search through the vtable and update overridden entries
   // Since check_signature_loaders acquires SystemDictionary_lock
   // which can block for gc, once we are in this loop, use handles
@@ -555,6 +546,7 @@
 }
 
 void klassVtable::put_method_at(Method* m, int index) {
+  assert(!m->is_private(), "private methods should not be in vtable");
   if (is_preinitialized_vtable()) {
     // At runtime initialize_vtable is rerun as part of link_class_impl()
     // for shared class loaded by the non-boot loader to obtain the loader
@@ -606,9 +598,11 @@
       // a final method never needs a new entry; final methods can be statically
       // resolved and they have to be present in the vtable only if they override
       // a super's method, in which case they re-use its entry
+      (target_method()->is_private()) ||
+      // private methods don't need to be in vtable
       (target_method()->is_static()) ||
       // static methods don't need to be in vtable
-      (target_method()->name() ==  vmSymbols::object_initializer_name())
+      (target_method()->name()->fast_compare(vmSymbols::object_initializer_name()) == 0)
       // <init> is never called dynamically-bound
       ) {
     return false;
@@ -619,7 +613,7 @@
   if (target_method()->method_holder() != NULL &&
       target_method()->method_holder()->is_interface()  &&
       !target_method()->is_abstract()) {
-    assert(target_method()->is_default_method() || target_method()->is_private(),
+    assert(target_method()->is_default_method(),
            "unexpected interface method type");
     return false;
   }
@@ -629,12 +623,6 @@
     return true;
   }
 
-  // private methods in classes always have a new entry in the vtable.
-  // Specification interpretation since classic has private methods not overriding.
-  if (target_method()->is_private()) {
-    return true;
-  }
-
   // Package private methods always need a new entry to root their own
   // overriding. This allows transitive overriding to work.
   if (target_method()->is_package_private()) {
@@ -660,8 +648,8 @@
     // make sure you use that class for is_override
     InstanceKlass* superk = super_method->method_holder();
     // we want only instance method matches
-    // pretend private methods are not in the super vtable
-    // since we do override around them: e.g. a.m pub/b.m private/c.m pub,
+    // ignore private methods found via lookup_method since they do not participate in overriding,
+    // and since we do override around them: e.g. a.m pub/b.m private/c.m pub,
     // ignore private, c.m pub does override a.m pub
     // For classes that were not javac'd together, we also do transitive overriding around
     // methods that have less accessibility