8161445: [BACKOUT] MemberNameTable doesn't purge stale entries
authorcoleenp
Mon, 01 Aug 2016 16:28:23 -0400
changeset 40102 23ff468d69b4
parent 40101 3b8101f0fd65
child 40103 3c3569845229
8161445: [BACKOUT] MemberNameTable doesn't purge stale entries Summary: Original change caused performance regression in microbenchmarks after GC Reviewed-by: dholmes, ecaspole
hotspot/src/share/vm/classfile/javaClasses.cpp
hotspot/src/share/vm/classfile/javaClasses.hpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/src/share/vm/prims/jvm.cpp
hotspot/src/share/vm/prims/methodHandles.cpp
hotspot/src/share/vm/prims/methodHandles.hpp
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/classfile/javaClasses.cpp	Mon Aug 01 16:28:23 2016 -0400
@@ -3243,15 +3243,6 @@
   mname->address_field_put(_vmindex_offset, (address) index);
 }
 
-bool java_lang_invoke_MemberName::equals(oop mn1, oop mn2) {
-  if (mn1 == mn2) {
-     return true;
-  }
-  return (vmtarget(mn1) == vmtarget(mn2) && flags(mn1) == flags(mn2) &&
-          vmindex(mn1) == vmindex(mn2) &&
-          clazz(mn1) == clazz(mn2));
-}
-
 oop java_lang_invoke_LambdaForm::vmentry(oop lform) {
   assert(is_instance(lform), "wrong type");
   return lform->obj_field(_vmentry_offset);
--- a/hotspot/src/share/vm/classfile/javaClasses.hpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/classfile/javaClasses.hpp	Mon Aug 01 16:28:23 2016 -0400
@@ -1107,8 +1107,6 @@
   static int flags_offset_in_bytes()            { return _flags_offset; }
   static int vmtarget_offset_in_bytes()         { return _vmtarget_offset; }
   static int vmindex_offset_in_bytes()          { return _vmindex_offset; }
-
-  static bool equals(oop mt1, oop mt2);
 };
 
 
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon Aug 01 16:28:23 2016 -0400
@@ -2737,7 +2737,7 @@
   return NULL;
 }
 
-oop InstanceKlass::add_member_name(Handle mem_name, bool intern) {
+bool InstanceKlass::add_member_name(Handle mem_name) {
   jweak mem_name_wref = JNIHandles::make_weak_global(mem_name);
   MutexLocker ml(MemberNameTable_lock);
   DEBUG_ONLY(NoSafepointVerifier nsv);
@@ -2747,7 +2747,7 @@
   // is called!
   Method* method = (Method*)java_lang_invoke_MemberName::vmtarget(mem_name());
   if (method->is_obsolete()) {
-    return NULL;
+    return false;
   } else if (method->is_old()) {
     // Replace method with redefined version
     java_lang_invoke_MemberName::set_vmtarget(mem_name(), method_with_idnum(method->method_idnum()));
@@ -2756,11 +2756,8 @@
   if (_member_names == NULL) {
     _member_names = new (ResourceObj::C_HEAP, mtClass) MemberNameTable(idnum_allocated_count());
   }
-  if (intern) {
-    return _member_names->find_or_add_member_name(mem_name_wref);
-  } else {
-    return _member_names->add_member_name(mem_name_wref);
-  }
+  _member_names->add_member_name(mem_name_wref);
+  return true;
 }
 
 // -----------------------------------------------------------------------------------------------------
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Mon Aug 01 16:28:23 2016 -0400
@@ -1306,7 +1306,7 @@
   // JSR-292 support
   MemberNameTable* member_names() { return _member_names; }
   void set_member_names(MemberNameTable* member_names) { _member_names = member_names; }
-  oop add_member_name(Handle member_name, bool intern);
+  bool add_member_name(Handle member_name);
 
 public:
   // JVMTI support
--- a/hotspot/src/share/vm/prims/jvm.cpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/prims/jvm.cpp	Mon Aug 01 16:28:23 2016 -0400
@@ -680,7 +680,7 @@
       // This can safepoint and redefine method, so need both new_obj and method
       // in a handle, for two different reasons.  new_obj can move, method can be
       // deleted if nothing is using it on the stack.
-      m->method_holder()->add_member_name(new_obj(), false);
+      m->method_holder()->add_member_name(new_obj());
     }
   }
 
--- a/hotspot/src/share/vm/prims/methodHandles.cpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/prims/methodHandles.cpp	Mon Aug 01 16:28:23 2016 -0400
@@ -178,7 +178,7 @@
   return NULL;
 }
 
-oop MethodHandles::init_method_MemberName(Handle mname, CallInfo& info, bool intern) {
+oop MethodHandles::init_method_MemberName(Handle mname, CallInfo& info) {
   assert(info.resolved_appendix().is_null(), "only normal methods here");
   methodHandle m = info.resolved_method();
   assert(m.not_null(), "null method handle");
@@ -279,7 +279,13 @@
   // If relevant, the vtable or itable value is stored as vmindex.
   // This is done eagerly, since it is readily available without
   // constructing any new objects.
-  return m->method_holder()->add_member_name(mname, intern);
+  // TO DO: maybe intern mname_oop
+  if (m->method_holder()->add_member_name(mname)) {
+    return mname();
+  } else {
+    // Redefinition caused this to fail.  Return NULL (and an exception?)
+    return NULL;
+  }
 }
 
 oop MethodHandles::init_field_MemberName(Handle mname, fieldDescriptor& fd, bool is_setter) {
@@ -969,9 +975,7 @@
         if (!java_lang_invoke_MemberName::is_instance(result()))
           return -99;  // caller bug!
         CallInfo info(m);
-        // Since this is going through the methods to create MemberNames, don't search
-        // for matching methods already in the table
-        oop saved = MethodHandles::init_method_MemberName(result, info, /*intern*/false);
+        oop saved = MethodHandles::init_method_MemberName(result, info);
         if (saved != result())
           results->obj_at_put(rfill-1, saved);  // show saved instance to user
       } else if (++overflow >= overflow_limit) {
@@ -1052,34 +1056,9 @@
   }
 }
 
-oop MemberNameTable::add_member_name(jweak mem_name_wref) {
+void MemberNameTable::add_member_name(jweak mem_name_wref) {
   assert_locked_or_safepoint(MemberNameTable_lock);
   this->push(mem_name_wref);
-  return JNIHandles::resolve(mem_name_wref);
-}
-
-oop MemberNameTable::find_or_add_member_name(jweak mem_name_wref) {
-  assert_locked_or_safepoint(MemberNameTable_lock);
-  oop new_mem_name = JNIHandles::resolve(mem_name_wref);
-
-  // Find matching member name in the list.
-  // This is linear because these because these are short lists.
-  int len = this->length();
-  int new_index = len;
-  for (int idx = 0; idx < len; idx++) {
-    oop mname = JNIHandles::resolve(this->at(idx));
-    if (mname == NULL) {
-      new_index = idx;
-      continue;
-    }
-    if (java_lang_invoke_MemberName::equals(new_mem_name, mname)) {
-      JNIHandles::destroy_weak_global(mem_name_wref);
-      return mname;
-    }
-  }
-  // Not found, push the new one, or reuse empty slot
-  this->at_put_grow(new_index, mem_name_wref);
-  return new_mem_name;
 }
 
 #if INCLUDE_JVMTI
--- a/hotspot/src/share/vm/prims/methodHandles.hpp	Mon Aug 01 15:38:58 2016 +0300
+++ b/hotspot/src/share/vm/prims/methodHandles.hpp	Mon Aug 01 16:28:23 2016 -0400
@@ -66,7 +66,7 @@
   static Handle new_MemberName(TRAPS);  // must be followed by init_MemberName
   static oop init_MemberName(Handle mname_h, Handle target_h); // compute vmtarget/vmindex from target
   static oop init_field_MemberName(Handle mname_h, fieldDescriptor& fd, bool is_setter = false);
-  static oop init_method_MemberName(Handle mname_h, CallInfo& info, bool intern = true);
+  static oop init_method_MemberName(Handle mname_h, CallInfo& info);
   static int method_ref_kind(Method* m, bool do_dispatch_if_possible = true);
   static int find_MemberNames(KlassHandle k, Symbol* name, Symbol* sig,
                               int mflags, KlassHandle caller,
@@ -235,8 +235,7 @@
  public:
   MemberNameTable(int methods_cnt);
   ~MemberNameTable();
-  oop add_member_name(jweak mem_name_ref);
-  oop find_or_add_member_name(jweak mem_name_ref);
+  void add_member_name(jweak mem_name_ref);
 
 #if INCLUDE_JVMTI
   // RedefineClasses() API support: