--- old/./src/share/vm/ci/ciEnv.cpp 2013-03-21 22:01:49.187054990 -0700 +++ new/./src/share/vm/ci/ciEnv.cpp 2013-03-21 22:01:48.982044824 -0700 @@ -483,7 +483,8 @@ { // We have to lock the cpool to keep the oop from being resolved // while we are accessing it. - MonitorLockerEx ml(cpool->lock()); + oop cplock = cpool->lock(); + ObjectLocker ol(cplock, THREAD, cplock != NULL); constantTag tag = cpool->tag_at(index); if (tag.is_klass()) { // The klass has been inserted into the constant pool --- old/./src/share/vm/classfile/classFileParser.cpp 2013-03-21 22:01:50.362113254 -0700 +++ new/./src/share/vm/classfile/classFileParser.cpp 2013-03-21 22:01:50.155102991 -0700 @@ -4091,10 +4091,11 @@ // Allocate mirror and initialize static fields java_lang_Class::create_mirror(this_klass, CHECK_(nullHandle)); - // Allocate a simple java object for locking during class initialization. + // Allocate a simple java object for (a) locking during class initialization; + // (b) locking of the class's constant pool. // This needs to be a java object because it can be held across a java call. typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK_NULL); - this_klass->set_init_lock(r); + this_klass->set_per_class_lock(r); // TODO: Move these oops to the mirror this_klass->set_protection_domain(protection_domain()); --- old/./src/share/vm/oops/constantPool.cpp 2013-03-21 22:01:51.523170824 -0700 +++ new/./src/share/vm/oops/constantPool.cpp 2013-03-21 22:01:51.306160065 -0700 @@ -40,6 +40,7 @@ #include "runtime/init.hpp" #include "runtime/javaCalls.hpp" #include "runtime/signature.hpp" +#include "runtime/synchronizer.hpp" #include "runtime/vframe.hpp" ConstantPool* ConstantPool::allocate(ClassLoaderData* loader_data, int length, TRAPS) { @@ -69,7 +70,6 @@ // only set to non-zero if constant pool is merged by RedefineClasses set_version(0); - set_lock(new Monitor(Monitor::nonleaf + 2, "A constant pool lock")); // initialize tag array int length = tags->length(); @@ -95,9 +95,6 @@ void ConstantPool::release_C_heap_structures() { // walk constant pool and decrement symbol reference counts unreference_symbols(); - - delete _lock; - set_lock(NULL); } objArrayOop ConstantPool::resolved_references() const { @@ -154,9 +151,6 @@ ClassLoaderData* loader_data = pool_holder()->class_loader_data(); set_resolved_references(loader_data->add_handle(refs_handle)); } - - // Also need to recreate the mutex. Make sure this matches the constructor - set_lock(new Monitor(Monitor::nonleaf + 2, "A constant pool lock")); } } @@ -167,7 +161,14 @@ set_resolved_reference_length( resolved_references() != NULL ? resolved_references()->length() : 0); set_resolved_references(NULL); - set_lock(NULL); +} + +oop ConstantPool::lock() { + if (_pool_holder) { + return _pool_holder->cp_lock(); + } else { + return NULL; + } } int ConstantPool::cp_to_object_index(int cp_index) { @@ -208,7 +209,9 @@ Symbol* name = NULL; Handle loader; - { MonitorLockerEx ml(this_oop->lock()); + { + oop cplock = this_oop->lock(); + ObjectLocker ol(cplock , THREAD, cplock != NULL); if (this_oop->tag_at(which).is_unresolved_klass()) { if (this_oop->tag_at(which).is_unresolved_klass_in_error()) { @@ -255,7 +258,8 @@ bool throw_orig_error = false; { - MonitorLockerEx ml(this_oop->lock()); + oop cplock = this_oop->lock(); + ObjectLocker ol(cplock, THREAD, cplock != NULL); // some other thread has beaten us and has resolved the class. if (this_oop->tag_at(which).is_klass()) { @@ -323,7 +327,8 @@ } return k(); } else { - MonitorLockerEx ml(this_oop->lock()); + oop cplock = this_oop->lock(); + ObjectLocker ol(cplock, THREAD, cplock != NULL); // Only updated constant pool - if it is resolved. do_resolve = this_oop->tag_at(which).is_unresolved_klass(); if (do_resolve) { @@ -619,7 +624,8 @@ int tag, TRAPS) { ResourceMark rm; Symbol* error = PENDING_EXCEPTION->klass()->name(); - MonitorLockerEx ml(this_oop->lock()); // lock cpool to change tag. + oop cplock = this_oop->lock(); + ObjectLocker ol(cplock, THREAD, cplock != NULL); // lock cpool to change tag. int error_tag = (tag == JVM_CONSTANT_MethodHandle) ? JVM_CONSTANT_MethodHandleInError : JVM_CONSTANT_MethodTypeInError; @@ -780,7 +786,8 @@ if (cache_index >= 0) { // Cache the oop here also. Handle result_handle(THREAD, result_oop); - MonitorLockerEx ml(this_oop->lock()); // don't know if we really need this + oop cplock = this_oop->lock(); + ObjectLocker ol(cplock, THREAD, cplock != NULL); // don't know if we really need this oop result = this_oop->resolved_references()->obj_at(cache_index); // Benign race condition: resolved_references may already be filled in while we were trying to lock. // The important thing here is that all threads pick up the same result. --- old/./src/share/vm/oops/constantPool.hpp 2013-03-21 22:01:52.697229039 -0700 +++ new/./src/share/vm/oops/constantPool.hpp 2013-03-21 22:01:52.481218329 -0700 @@ -111,7 +111,6 @@ int _version; } _saved; - Monitor* _lock; void set_tags(Array* tags) { _tags = tags; } void tag_at_put(int which, jbyte t) { tags()->at_put(which, t); } @@ -782,8 +781,11 @@ void set_resolved_reference_length(int length) { _saved._resolved_reference_length = length; } int resolved_reference_length() const { return _saved._resolved_reference_length; } - void set_lock(Monitor* lock) { _lock = lock; } - Monitor* lock() { return _lock; } + + // May be null, in which case locking is not necessary. Use it like this: + // oop cplock = cp->lock(); + // ObjectLocker ol(cplock , THREAD, cplock != NULL); + oop lock(); // Decrease ref counts of symbols that are in the constant pool // when the holder class is unloaded --- old/./src/share/vm/oops/cpCache.cpp 2013-03-21 22:01:53.811284280 -0700 +++ new/./src/share/vm/oops/cpCache.cpp 2013-03-21 22:01:53.595273570 -0700 @@ -266,7 +266,8 @@ // the lock, so that when the losing writer returns, he can use the linked // cache entry. - MonitorLockerEx ml(cpool->lock()); + oop cplock = cpool->lock(); + ObjectLocker ol(cplock, Thread::current(), cplock != NULL); if (!is_f1_null()) { return; } --- old/./src/share/vm/oops/instanceKlass.cpp 2013-03-21 22:01:54.894337982 -0700 +++ new/./src/share/vm/oops/instanceKlass.cpp 2013-03-21 22:01:54.688327768 -0700 @@ -278,7 +278,7 @@ set_is_marked_dependent(false); set_init_state(InstanceKlass::allocated); set_init_thread(NULL); - set_init_lock(NULL); + set_per_class_lock(NULL); set_reference_type(rt); set_oop_map_cache(NULL); set_jni_ids(NULL); @@ -411,32 +411,13 @@ // alive once this InstanceKlass is deallocated. set_protection_domain(NULL); set_signers(NULL); - set_init_lock(NULL); + set_per_class_lock(NULL); // We should deallocate the Annotations instance MetadataFactory::free_metadata(loader_data, annotations()); set_annotations(NULL); } -volatile oop InstanceKlass::init_lock() const { - volatile oop lock = _init_lock; // read once - assert((oop)lock != NULL || !is_not_initialized(), // initialized or in_error state - "only fully initialized state can have a null lock"); - return lock; -} - -// Set the initialization lock to null so the object can be GC'ed. Any racing -// threads to get this lock will see a null lock and will not lock. -// That's okay because they all check for initialized state after getting -// the lock and return. -void InstanceKlass::fence_and_clear_init_lock() { - // make sure previous stores are all done, notably the init_state. - OrderAccess::storestore(); - klass_oop_store(&_init_lock, NULL); - assert(!is_not_initialized(), "class must be initialized now"); -} - - bool InstanceKlass::should_be_initialized() const { return !is_initialized(); } @@ -473,7 +454,7 @@ void InstanceKlass::eager_initialize_impl(instanceKlassHandle this_oop) { EXCEPTION_MARK; volatile oop init_lock = this_oop->init_lock(); - ObjectLocker ol(init_lock, THREAD, init_lock != NULL); + ObjectLocker ol(init_lock, THREAD); // abort if someone beat us to the initialization if (!this_oop->is_not_initialized()) return; // note: not equivalent to is_initialized() @@ -492,7 +473,6 @@ } else { // linking successfull, mark class as initialized this_oop->set_init_state (fully_initialized); - this_oop->fence_and_clear_init_lock(); // trace if (TraceClassInitialization) { ResourceMark rm(THREAD); @@ -619,7 +599,7 @@ // verification & rewriting { volatile oop init_lock = this_oop->init_lock(); - ObjectLocker ol(init_lock, THREAD, init_lock != NULL); + ObjectLocker ol(init_lock, THREAD); // rewritten will have been set if loader constraint error found // on an earlier link attempt // don't verify or rewrite if already rewritten @@ -742,7 +722,7 @@ // Step 1 { volatile oop init_lock = this_oop->init_lock(); - ObjectLocker ol(init_lock, THREAD, init_lock != NULL); + ObjectLocker ol(init_lock, THREAD); Thread *self = THREAD; // it's passed the current thread @@ -890,9 +870,8 @@ void InstanceKlass::set_initialization_state_and_notify_impl(instanceKlassHandle this_oop, ClassState state, TRAPS) { volatile oop init_lock = this_oop->init_lock(); - ObjectLocker ol(init_lock, THREAD, init_lock != NULL); + ObjectLocker ol(init_lock, THREAD); this_oop->set_init_state(state); - this_oop->fence_and_clear_init_lock(); ol.notify_all(CHECK); } @@ -1908,7 +1887,7 @@ cl->do_oop(adr_protection_domain()); cl->do_oop(adr_signers()); - cl->do_oop(adr_init_lock()); + cl->do_oop(adr_per_class_lock()); // Don't walk the arrays since they are walked from the ClassLoaderData objects. } @@ -2273,7 +2252,7 @@ } // Need to reinstate when reading back the class. - set_init_lock(NULL); + set_per_class_lock(NULL); // do array classes also. array_klasses_do(remove_unshareable_in_class); @@ -2311,7 +2290,7 @@ // it can be held across a java call. typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK); Handle h(THREAD, (oop)r); - ik->set_init_lock(h()); + ik->set_per_class_lock(h()); // restore constant pool resolved references ik->constants()->restore_unshareable_info(CHECK); @@ -2831,7 +2810,7 @@ st->print(BULLET"protection domain: "); ((InstanceKlass*)this)->protection_domain()->print_value_on(st); st->cr(); st->print(BULLET"host class: "); host_klass()->print_value_on_maybe_null(st); st->cr(); st->print(BULLET"signers: "); signers()->print_value_on(st); st->cr(); - st->print(BULLET"init_lock: "); ((oop)_init_lock)->print_value_on(st); st->cr(); + st->print(BULLET"per_class_lock: "); ((oop)_per_class_lock)->print_value_on(st); st->cr(); if (source_file_name() != NULL) { st->print(BULLET"source file: "); source_file_name()->print_value_on(st); --- old/./src/share/vm/oops/instanceKlass.hpp 2013-03-21 22:01:56.031394363 -0700 +++ new/./src/share/vm/oops/instanceKlass.hpp 2013-03-21 22:01:55.825384148 -0700 @@ -183,10 +183,11 @@ oop _protection_domain; // Class signers. objArrayOop _signers; - // Initialization lock. Must be one per class and it has to be a VM internal - // object so java code cannot lock it (like the mirror) + // Lock for (1) initialization; (2) access to the ConstantPool of this class. + // Must be one per class and it has to be a VM internal object so java code + // cannot lock it (like the mirror). // It has to be an object not a Mutex because it's held through java calls. - volatile oop _init_lock; + volatile oop _per_class_lock; // Annotations for this class Annotations* _annotations; @@ -966,6 +967,9 @@ #endif // INCLUDE_ALL_GCS u2 idnum_allocated_count() const { return _idnum_allocated_count; } + + oop cp_lock() const { return _per_class_lock; } + private: // initialization state #ifdef ASSERT @@ -992,14 +996,13 @@ { OrderAccess::release_store_ptr(&_methods_cached_itable_indices, indices); } // Lock during initialization - volatile oop init_lock() const; - void set_init_lock(oop value) { klass_oop_store(&_init_lock, value); } - void fence_and_clear_init_lock(); // after fully_initialized + volatile oop init_lock() const {return _per_class_lock; } + void set_per_class_lock(oop value) { klass_oop_store(&_per_class_lock, value); } // Offsets for memory management oop* adr_protection_domain() const { return (oop*)&this->_protection_domain;} oop* adr_signers() const { return (oop*)&this->_signers;} - oop* adr_init_lock() const { return (oop*)&this->_init_lock;} + oop* adr_per_class_lock() const { return (oop*)&this->_per_class_lock;} // Static methods that are used to implement member methods where an exposed this pointer // is needed due to possible GCs --- old/./src/share/vm/prims/jvmtiEnv.cpp 2013-03-21 22:01:57.100447371 -0700 +++ new/./src/share/vm/prims/jvmtiEnv.cpp 2013-03-21 22:01:56.911437999 -0700 @@ -259,7 +259,8 @@ // bytes to the InstanceKlass here because they have not been // validated and we're not at a safepoint. constantPoolHandle constants(current_thread, ikh->constants()); - MonitorLockerEx ml(constants->lock()); // lock constant pool while we query it + oop cplock = constants->lock(); + ObjectLocker ol(cplock, current_thread, cplock != NULL); // lock constant pool while we query it JvmtiClassFileReconstituter reconstituter(ikh); if (reconstituter.get_error() != JVMTI_ERROR_NONE) { @@ -2417,7 +2418,8 @@ instanceKlassHandle ikh(thread, k_oop); constantPoolHandle constants(thread, ikh->constants()); - MonitorLockerEx ml(constants->lock()); // lock constant pool while we query it + oop cplock = constants->lock(); + ObjectLocker ol(cplock, thread, cplock != NULL); // lock constant pool while we query it JvmtiConstantPoolReconstituter reconstituter(ikh); if (reconstituter.get_error() != JVMTI_ERROR_NONE) {