# HG changeset patch # User dholmes # Date 1472516025 14400 # Node ID 8d35e19f55484f6ccc2ca19114e941041474323c # Parent 98cb935dc074553a889e00eae2e9a7e2a68346f4 8158854: Ensure release_store is paired with load_acquire in lock-free code Reviewed-by: shade, dcubed, zgu diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/classfile/classLoader.hpp --- a/hotspot/src/share/vm/classfile/classLoader.hpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/classfile/classLoader.hpp Mon Aug 29 20:13:45 2016 -0400 @@ -50,12 +50,14 @@ class ClassPathEntry : public CHeapObj { private: - ClassPathEntry* _next; + ClassPathEntry* volatile _next; public: // Next entry in class path - ClassPathEntry* next() const { return _next; } + ClassPathEntry* next() const { + return (ClassPathEntry*) OrderAccess::load_ptr_acquire(&_next); + } void set_next(ClassPathEntry* next) { - // may have unlocked readers, so write atomically. + // may have unlocked readers, so ensure visibility. OrderAccess::release_store_ptr(&_next, next); } virtual bool is_jrt() = 0; diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/classfile/verifier.cpp --- a/hotspot/src/share/vm/classfile/verifier.cpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/classfile/verifier.cpp Mon Aug 29 20:13:45 2016 -0400 @@ -67,12 +67,12 @@ static volatile jint _is_new_verify_byte_codes_fn = (jint) true; static void* verify_byte_codes_fn() { - if (_verify_byte_codes_fn == NULL) { + if (OrderAccess::load_ptr_acquire(&_verify_byte_codes_fn) == NULL) { void *lib_handle = os::native_java_library(); void *func = os::dll_lookup(lib_handle, "VerifyClassCodesForMajorVersion"); OrderAccess::release_store_ptr(&_verify_byte_codes_fn, func); if (func == NULL) { - OrderAccess::release_store(&_is_new_verify_byte_codes_fn, false); + _is_new_verify_byte_codes_fn = false; func = os::dll_lookup(lib_handle, "VerifyClassCodes"); OrderAccess::release_store_ptr(&_verify_byte_codes_fn, func); } diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/arrayKlass.hpp --- a/hotspot/src/share/vm/oops/arrayKlass.hpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/arrayKlass.hpp Mon Aug 29 20:13:45 2016 -0400 @@ -56,7 +56,9 @@ void set_dimension(int dimension) { _dimension = dimension; } Klass* higher_dimension() const { return _higher_dimension; } + inline Klass* higher_dimension_acquire() const; // load with acquire semantics void set_higher_dimension(Klass* k) { _higher_dimension = k; } + inline void release_set_higher_dimension(Klass* k); // store with release semantics Klass** adr_higher_dimension() { return (Klass**)&this->_higher_dimension;} Klass* lower_dimension() const { return _lower_dimension; } diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/arrayKlass.inline.hpp --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/hotspot/src/share/vm/oops/arrayKlass.inline.hpp Mon Aug 29 20:13:45 2016 -0400 @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +#ifndef SHARE_VM_OOPS_ARRAYKLASS_INLINE_HPP +#define SHARE_VM_OOPS_ARRAYKLASS_INLINE_HPP + +#include "runtime/orderAccess.inline.hpp" +#include "oops/arrayKlass.hpp" + +inline Klass* ArrayKlass::higher_dimension_acquire() const { + return (Klass*) OrderAccess::load_ptr_acquire(&_higher_dimension); +} + +inline void ArrayKlass::release_set_higher_dimension(Klass* k) { + OrderAccess::release_store_ptr(&_higher_dimension, k); +} + +#endif // SHARE_VM_OOPS_ARRAYKLASS_INLINE_HPP diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/instanceKlass.cpp --- a/hotspot/src/share/vm/oops/instanceKlass.cpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Mon Aug 29 20:13:45 2016 -0400 @@ -1041,7 +1041,8 @@ } Klass* InstanceKlass::array_klass_impl(instanceKlassHandle this_k, bool or_null, int n, TRAPS) { - if (this_k->array_klasses() == NULL) { + // Need load-acquire for lock-free read + if (this_k->array_klasses_acquire() == NULL) { if (or_null) return NULL; ResourceMark rm; @@ -1054,7 +1055,8 @@ // Check if update has already taken place if (this_k->array_klasses() == NULL) { Klass* k = ObjArrayKlass::allocate_objArray_klass(this_k->class_loader_data(), 1, this_k, CHECK_NULL); - this_k->set_array_klasses(k); + // use 'release' to pair with lock-free load + this_k->release_set_array_klasses(k); } } } diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/instanceKlass.hpp --- a/hotspot/src/share/vm/oops/instanceKlass.hpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Mon Aug 29 20:13:45 2016 -0400 @@ -148,7 +148,7 @@ // Package this class is defined in PackageEntry* _package_entry; // Array classes holding elements of this class. - Klass* _array_klasses; + Klass* volatile _array_klasses; // Constant pool for this class. ConstantPool* _constants; // The InnerClasses attribute and EnclosingMethod attribute. The @@ -230,7 +230,7 @@ OopMapCache* volatile _oop_map_cache; // OopMapCache for all methods in the klass (allocated lazily) MemberNameTable* _member_names; // Member names JNIid* _jni_ids; // First JNI identifier for static fields in this class - jmethodID* _methods_jmethod_ids; // jmethodIDs corresponding to method_idnum, or NULL if none + jmethodID* volatile _methods_jmethod_ids; // jmethodIDs corresponding to method_idnum, or NULL if none intptr_t _dep_context; // packed DependencyContext structure nmethod* _osr_nmethods_head; // Head of list of on-stack replacement nmethods for this class #if INCLUDE_JVMTI @@ -368,7 +368,9 @@ // array klasses Klass* array_klasses() const { return _array_klasses; } + inline Klass* array_klasses_acquire() const; // load with acquire semantics void set_array_klasses(Klass* k) { _array_klasses = k; } + inline void release_set_array_klasses(Klass* k); // store with release semantics // methods Array* methods() const { return _methods; } @@ -1238,10 +1240,8 @@ // cache management logic if the caches can grow instead of just // going from NULL to non-NULL. bool idnum_can_increment() const { return has_been_redefined(); } - jmethodID* methods_jmethod_ids_acquire() const - { return (jmethodID*)OrderAccess::load_ptr_acquire(&_methods_jmethod_ids); } - void release_set_methods_jmethod_ids(jmethodID* jmeths) - { OrderAccess::release_store_ptr(&_methods_jmethod_ids, jmeths); } + inline jmethodID* methods_jmethod_ids_acquire() const; + inline void release_set_methods_jmethod_ids(jmethodID* jmeths); // Lock during initialization public: diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/instanceKlass.inline.hpp --- a/hotspot/src/share/vm/oops/instanceKlass.inline.hpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/instanceKlass.inline.hpp Mon Aug 29 20:13:45 2016 -0400 @@ -29,10 +29,27 @@ #include "oops/instanceKlass.hpp" #include "oops/klass.hpp" #include "oops/oop.inline.hpp" +#include "runtime/orderAccess.inline.hpp" #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" +inline Klass* InstanceKlass::array_klasses_acquire() const { + return (Klass*) OrderAccess::load_ptr_acquire(&_array_klasses); +} + +inline void InstanceKlass::release_set_array_klasses(Klass* k) { + OrderAccess::release_store_ptr(&_array_klasses, k); +} + +inline jmethodID* InstanceKlass::methods_jmethod_ids_acquire() const { + return (jmethodID*)OrderAccess::load_ptr_acquire(&_methods_jmethod_ids); +} + +inline void InstanceKlass::release_set_methods_jmethod_ids(jmethodID* jmeths) { + OrderAccess::release_store_ptr(&_methods_jmethod_ids, jmeths); +} + // The iteration over the oops in objects is a hot path in the GC code. // By force inlining the following functions, we get similar GC performance // as the previous macro based implementation. diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/objArrayKlass.cpp --- a/hotspot/src/share/vm/oops/objArrayKlass.cpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/objArrayKlass.cpp Mon Aug 29 20:13:45 2016 -0400 @@ -34,6 +34,7 @@ #include "memory/metadataFactory.hpp" #include "memory/resourceArea.hpp" #include "memory/universe.inline.hpp" +#include "oops/arrayKlass.inline.hpp" #include "oops/instanceKlass.hpp" #include "oops/klass.inline.hpp" #include "oops/objArrayKlass.inline.hpp" @@ -42,7 +43,6 @@ #include "oops/symbol.hpp" #include "runtime/handles.inline.hpp" #include "runtime/mutexLocker.hpp" -#include "runtime/orderAccess.inline.hpp" #include "utilities/copy.hpp" #include "utilities/macros.hpp" @@ -321,7 +321,8 @@ int dim = dimension(); if (dim == n) return this; - if (higher_dimension() == NULL) { + // lock-free read needs acquire semantics + if (higher_dimension_acquire() == NULL) { if (or_null) return NULL; ResourceMark rm; @@ -339,8 +340,8 @@ ObjArrayKlass::allocate_objArray_klass(class_loader_data(), dim + 1, this, CHECK_NULL); ObjArrayKlass* ak = ObjArrayKlass::cast(k); ak->set_lower_dimension(this); - OrderAccess::storestore(); - set_higher_dimension(ak); + // use 'release' to pair with lock-free load + release_set_higher_dimension(ak); assert(ak->is_objArray_klass(), "incorrect initialization of ObjArrayKlass"); } } diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/oops/typeArrayKlass.cpp --- a/hotspot/src/share/vm/oops/typeArrayKlass.cpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/oops/typeArrayKlass.cpp Mon Aug 29 20:13:45 2016 -0400 @@ -34,6 +34,7 @@ #include "memory/resourceArea.hpp" #include "memory/universe.hpp" #include "memory/universe.inline.hpp" +#include "oops/arrayKlass.inline.hpp" #include "oops/instanceKlass.hpp" #include "oops/klass.inline.hpp" #include "oops/objArrayKlass.hpp" @@ -41,7 +42,6 @@ #include "oops/typeArrayKlass.inline.hpp" #include "oops/typeArrayOop.inline.hpp" #include "runtime/handles.inline.hpp" -#include "runtime/orderAccess.inline.hpp" #include "utilities/macros.hpp" bool TypeArrayKlass::compute_is_subtype_of(Klass* k) { @@ -166,7 +166,8 @@ if (dim == n) return this; - if (higher_dimension() == NULL) { + // lock-free read needs acquire semantics + if (higher_dimension_acquire() == NULL) { if (or_null) return NULL; ResourceMark rm; @@ -181,8 +182,8 @@ class_loader_data(), dim + 1, this, CHECK_NULL); ObjArrayKlass* h_ak = ObjArrayKlass::cast(oak); h_ak->set_lower_dimension(this); - OrderAccess::storestore(); - set_higher_dimension(h_ak); + // use 'release' to pair with lock-free load + release_set_higher_dimension(h_ak); assert(h_ak->is_objArray_klass(), "incorrect initialization of ObjArrayKlass"); } } diff -r 98cb935dc074 -r 8d35e19f5548 hotspot/src/share/vm/runtime/vmStructs.cpp --- a/hotspot/src/share/vm/runtime/vmStructs.cpp Mon Aug 29 23:04:48 2016 +0400 +++ b/hotspot/src/share/vm/runtime/vmStructs.cpp Mon Aug 29 20:13:45 2016 -0400 @@ -242,7 +242,7 @@ nonstatic_field(ConstantPool, _reference_map, Array*) \ nonstatic_field(ConstantPoolCache, _length, int) \ nonstatic_field(ConstantPoolCache, _constant_pool, ConstantPool*) \ - nonstatic_field(InstanceKlass, _array_klasses, Klass*) \ + volatile_nonstatic_field(InstanceKlass, _array_klasses, Klass*) \ nonstatic_field(InstanceKlass, _methods, Array*) \ nonstatic_field(InstanceKlass, _default_methods, Array*) \ nonstatic_field(InstanceKlass, _local_interfaces, Array*) \ @@ -271,7 +271,7 @@ nonstatic_field(InstanceKlass, _osr_nmethods_head, nmethod*) \ JVMTI_ONLY(nonstatic_field(InstanceKlass, _breakpoints, BreakpointInfo*)) \ nonstatic_field(InstanceKlass, _generic_signature_index, u2) \ - nonstatic_field(InstanceKlass, _methods_jmethod_ids, jmethodID*) \ + volatile_nonstatic_field(InstanceKlass, _methods_jmethod_ids, jmethodID*) \ volatile_nonstatic_field(InstanceKlass, _idnum_allocated_count, u2) \ nonstatic_field(InstanceKlass, _annotations, Annotations*) \ nonstatic_field(InstanceKlass, _method_ordering, Array*) \