changeset 3910:30866cd626b0

8004883: NPG: clean up anonymous class fix Summary: Add klass_holder() to return either mirror or class_loader depending on if the class is anonymous or not. Reviewed-by: stefank, jrose
author coleenp
date Wed, 12 Dec 2012 11:39:29 -0500
parents 4a2ed49abd51
children 18712b1caf7a
files src/share/vm/asm/codeBuffer.cpp src/share/vm/classfile/classLoaderData.cpp src/share/vm/classfile/classLoaderData.hpp src/share/vm/compiler/compileBroker.cpp src/share/vm/oops/instanceKlass.hpp src/share/vm/oops/klass.hpp
diffstat 6 files changed, 24 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/asm/codeBuffer.cpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/asm/codeBuffer.cpp	Wed Dec 12 11:39:29 2012 -0500
@@ -496,21 +496,9 @@
   dest->verify_section_allocation();
 }
 
-// Anonymous classes need mirror to keep the metadata alive but
-// for regular classes, the class_loader is sufficient.
+// Append an oop reference that keeps the class alive.
 static void append_oop_references(GrowableArray<oop>* oops, Klass* k) {
-  if (k->oop_is_instance()) {
-    InstanceKlass* ik = InstanceKlass::cast(k);
-    if (ik->is_anonymous()) {
-      oop o = ik->java_mirror();
-      assert (o != NULL, "should have a mirror");
-      if (!oops->contains(o)) {
-        oops->append(o);
-      }
-      return;  // only need the mirror
-    }
-  }
-  oop cl = k->class_loader();
+  oop cl = k->klass_holder();
   if (cl != NULL && !oops->contains(cl)) {
     oops->append(cl);
   }
--- a/src/share/vm/classfile/classLoaderData.cpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/classfile/classLoaderData.cpp	Wed Dec 12 11:39:29 2012 -0500
@@ -64,8 +64,10 @@
 
 ClassLoaderData * ClassLoaderData::_the_null_class_loader_data = NULL;
 
-ClassLoaderData::ClassLoaderData(Handle h_class_loader) : _class_loader(h_class_loader()),
-  _metaspace(NULL), _unloading(false), _keep_alive(false), _klasses(NULL),
+ClassLoaderData::ClassLoaderData(Handle h_class_loader, bool is_anonymous) :
+  _class_loader(h_class_loader()),
+  _is_anonymous(is_anonymous), _keep_alive(is_anonymous), // initially
+  _metaspace(NULL), _unloading(false), _klasses(NULL),
   _claimed(0), _jmethod_ids(NULL), _handles(NULL), _deallocate_list(NULL),
   _next(NULL), _dependencies(NULL),
   _metaspace_lock(new Mutex(Monitor::leaf+1, "Metaspace allocation lock", true)) {
@@ -257,13 +259,6 @@
   ShouldNotReachHere();   // should have found this class!!
 }
 
-
-bool ClassLoaderData::is_anonymous() const {
-  Klass* k = _klasses;
-  return (_keep_alive || (k != NULL && k->oop_is_instance() &&
-          InstanceKlass::cast(k)->is_anonymous()));
-}
-
 void ClassLoaderData::unload() {
   _unloading = true;
 
@@ -396,8 +391,7 @@
 // These anonymous class loaders are to contain classes used for JSR292
 ClassLoaderData* ClassLoaderData::anonymous_class_loader_data(oop loader, TRAPS) {
   // Add a new class loader data to the graph.
-  ClassLoaderData* cld = ClassLoaderDataGraph::add(NULL, loader, CHECK_NULL);
-  return cld;
+  return ClassLoaderDataGraph::add(NULL, loader, CHECK_NULL);
 }
 
 const char* ClassLoaderData::loader_name() {
@@ -475,7 +469,9 @@
   // Create one.
   ClassLoaderData* *list_head = &_head;
   ClassLoaderData* next = _head;
-  ClassLoaderData* cld = new ClassLoaderData(loader);
+
+  bool is_anonymous = (cld_addr == NULL);
+  ClassLoaderData* cld = new ClassLoaderData(loader, is_anonymous);
 
   if (cld_addr != NULL) {
     // First, Atomically set it
@@ -485,10 +481,6 @@
       // Returns the data.
       return old;
     }
-  } else {
-    // Disallow unloading for this CLD during initialization if there is no
-    // class_loader oop to link this to.
-    cld->set_keep_alive(true);
   }
 
   // We won the race, and therefore the task of adding the data to the list of
--- a/src/share/vm/classfile/classLoaderData.hpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/classfile/classLoaderData.hpp	Wed Dec 12 11:39:29 2012 -0500
@@ -109,6 +109,7 @@
   Mutex* _metaspace_lock;  // Locks the metaspace for allocations and setup.
   bool _unloading;         // true if this class loader goes away
   bool _keep_alive;        // if this CLD can be unloaded for anonymous loaders
+  bool _is_anonymous;      // if this CLD is for an anonymous class
   volatile int _claimed;   // true if claimed, for example during GC traces.
                            // To avoid applying oop closure more than once.
                            // Has to be an int because we cas it.
@@ -139,7 +140,7 @@
   void set_next(ClassLoaderData* next) { _next = next; }
   ClassLoaderData* next() const        { return _next; }
 
-  ClassLoaderData(Handle h_class_loader);
+  ClassLoaderData(Handle h_class_loader, bool is_anonymous);
   ~ClassLoaderData();
 
   void set_metaspace(Metaspace* m) { _metaspace = m; }
@@ -174,12 +175,12 @@
     return _the_null_class_loader_data;
   }
 
-  bool is_anonymous() const;
+  bool is_anonymous() const { return _is_anonymous; }
 
   static void init_null_class_loader_data() {
     assert(_the_null_class_loader_data == NULL, "cannot initialize twice");
     assert(ClassLoaderDataGraph::_head == NULL, "cannot initialize twice");
-    _the_null_class_loader_data = new ClassLoaderData((oop)NULL);
+    _the_null_class_loader_data = new ClassLoaderData((oop)NULL, false);
     ClassLoaderDataGraph::_head = _the_null_class_loader_data;
     assert(_the_null_class_loader_data->is_the_null_class_loader_data(), "Must be");
     if (DumpSharedSpaces) {
--- a/src/share/vm/compiler/compileBroker.cpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/compiler/compileBroker.cpp	Wed Dec 12 11:39:29 2012 -0500
@@ -269,12 +269,10 @@
                              const char* comment,
                              bool is_blocking) {
   assert(!_lock->is_locked(), "bad locking");
-  InstanceKlass* holder = method->method_holder();
 
   _compile_id = compile_id;
   _method = method();
-  _method_holder = JNIHandles::make_global(
-        holder->is_anonymous() ? holder->java_mirror(): holder->class_loader());
+  _method_holder = JNIHandles::make_global(method->method_holder()->klass_holder());
   _osr_bci = osr_bci;
   _is_blocking = is_blocking;
   _comp_level = comp_level;
@@ -298,10 +296,7 @@
       } else {
         _hot_method = hot_method();
         // only add loader or mirror if different from _method_holder
-        InstanceKlass* hot_holder = hot_method->method_holder();
-        _hot_method_holder = JNIHandles::make_global(
-               hot_holder->is_anonymous() ? hot_holder->java_mirror() :
-                                            hot_holder->class_loader());
+        _hot_method_holder = JNIHandles::make_global(hot_method->method_holder()->klass_holder());
       }
     }
   }
--- a/src/share/vm/oops/instanceKlass.hpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/oops/instanceKlass.hpp	Wed Dec 12 11:39:29 2012 -0500
@@ -538,6 +538,12 @@
     }
   }
 
+  // Oop that keeps the metadata for this class from being unloaded
+  // in places where the metadata is stored in other places, like nmethods
+  oop klass_holder() const {
+    return is_anonymous() ? java_mirror() : class_loader();
+  }
+
   // signers
   objArrayOop signers() const              { return _signers; }
   void set_signers(objArrayOop s)          { klass_oop_store((oop*)&_signers, s); }
--- a/src/share/vm/oops/klass.hpp	Fri Dec 07 10:55:16 2012 -0800
+++ b/src/share/vm/oops/klass.hpp	Wed Dec 12 11:39:29 2012 -0500
@@ -451,6 +451,8 @@
 
   oop class_loader() const;
 
+  virtual oop klass_holder() const      { return class_loader(); }
+
  protected:
   virtual Klass* array_klass_impl(bool or_null, int rank, TRAPS);
   virtual Klass* array_klass_impl(bool or_null, TRAPS);