8158854: Ensure release_store is paired with load_acquire in lock-free code

Reviewed-by: shade, dcubed, zgu
This commit is contained in:
David Holmes 2016-08-29 20:13:45 -04:00
parent 56ff858c45
commit 6db26ca5bf
10 changed files with 87 additions and 23 deletions

View File

@ -50,12 +50,14 @@ class ClassFileStream;
class ClassPathEntry : public CHeapObj<mtClass> {
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;

View File

@ -67,12 +67,12 @@ static void* volatile _verify_byte_codes_fn = NULL;
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);
}

View File

@ -56,7 +56,9 @@ class ArrayKlass: public Klass {
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; }

View File

@ -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

View File

@ -1041,7 +1041,8 @@ Klass* InstanceKlass::array_klass_impl(bool or_null, int n, TRAPS) {
}
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 @@ Klass* InstanceKlass::array_klass_impl(instanceKlassHandle this_k, bool or_null,
// 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);
}
}
}

View File

@ -148,7 +148,7 @@ class InstanceKlass: public Klass {
// 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 @@ class InstanceKlass: public Klass {
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 @@ class InstanceKlass: public Klass {
// 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<Method*>* methods() const { return _methods; }
@ -1238,10 +1240,8 @@ private:
// 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:

View File

@ -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.

View File

@ -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 @@ Klass* ObjArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
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 @@ Klass* ObjArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
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");
}
}

View File

@ -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 @@ Klass* TypeArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
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 @@ Klass* TypeArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
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");
}
}

View File

@ -242,7 +242,7 @@ typedef CompactHashtable<Symbol*, char> SymbolCompactHashTable;
nonstatic_field(ConstantPool, _reference_map, Array<u2>*) \
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<Method*>*) \
nonstatic_field(InstanceKlass, _default_methods, Array<Method*>*) \
nonstatic_field(InstanceKlass, _local_interfaces, Array<Klass*>*) \
@ -271,7 +271,7 @@ typedef CompactHashtable<Symbol*, char> SymbolCompactHashTable;
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<int>*) \