diff --git a/src/share/vm/ci/ciMethodData.cpp b/src/share/vm/ci/ciMethodData.cpp index 950e3d6dd212a85f3dc93a907e6c3a4f6cb39b09..bc8794063ad18954076e959cd27d443d3b7af1f6 100644 --- a/src/share/vm/ci/ciMethodData.cpp +++ b/src/share/vm/ci/ciMethodData.cpp @@ -87,8 +87,9 @@ void ciMethodData::load_extra_data() { DataLayout* dp_dst = extra_data_base(); for (;; dp_src = MethodData::next_extra(dp_src), dp_dst = MethodData::next_extra(dp_dst)) { assert(dp_src < end_src, "moved past end of extra data"); - assert(dp_src->tag() == dp_dst->tag(), err_msg("should be same tags %d != %d", dp_src->tag(), dp_dst->tag())); - switch(dp_src->tag()) { + // New traps in the MDO can be added as we translate the copy so + // look at the entries in the copy. + switch(dp_dst->tag()) { case DataLayout::speculative_trap_data_tag: { ciSpeculativeTrapData* data_dst = new ciSpeculativeTrapData(dp_dst); SpeculativeTrapData* data_src = new SpeculativeTrapData(dp_src); @@ -102,7 +103,7 @@ void ciMethodData::load_extra_data() { // An empty slot or ArgInfoData entry marks the end of the trap data return; default: - fatal(err_msg("bad tag = %d", dp_src->tag())); + fatal(err_msg("bad tag = %d", dp_dst->tag())); } } } diff --git a/src/share/vm/oops/methodData.cpp b/src/share/vm/oops/methodData.cpp index 59b7c504d290d0f999d9829ad78cb3c627bf4862..33551cb023f57c359b80715733016fae92496267 100644 --- a/src/share/vm/oops/methodData.cpp +++ b/src/share/vm/oops/methodData.cpp @@ -1066,7 +1066,8 @@ void MethodData::post_initialize(BytecodeStream* stream) { } // Initialize the MethodData* corresponding to a given method. -MethodData::MethodData(methodHandle method, int size, TRAPS) { +MethodData::MethodData(methodHandle method, int size, TRAPS) + : _extra_data_lock(Monitor::leaf, "MDO extra data lock") { No_Safepoint_Verifier no_safepoint; // init function atomic wrt GC ResourceMark rm; // Set the method back-pointer. @@ -1230,7 +1231,7 @@ DataLayout* MethodData::next_extra(DataLayout* dp) { return (DataLayout*)((address)dp + DataLayout::compute_size_in_bytes(nb_cells)); } -ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp) { +ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent) { DataLayout* end = extra_data_limit(); for (;; dp = next_extra(dp)) { @@ -1252,10 +1253,11 @@ ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout if (m != NULL) { SpeculativeTrapData* data = new SpeculativeTrapData(dp); // data->method() may be null in case of a concurrent - // allocation. Assume it's for the same method and use that + // allocation. Maybe it's for the same method. Try to use that // entry in that case. if (dp->bci() == bci) { if (data->method() == NULL) { + assert(concurrent, "impossible because no concurrent allocation"); return NULL; } else if (data->method() == m) { return data; @@ -1284,40 +1286,40 @@ ProfileData* MethodData::bci_to_extra_data(int bci, Method* m, bool create_if_mi // Allocation in the extra data space has to be atomic because not // all entries have the same size and non atomic concurrent // allocation would result in a corrupted extra data space. - while (true) { - ProfileData* result = bci_to_extra_data_helper(bci, m, dp); - if (result != NULL) { + ProfileData* result = bci_to_extra_data_helper(bci, m, dp, true); + if (result != NULL) { + return result; + } + + if (create_if_missing && dp < end) { + MutexLocker ml(&_extra_data_lock); + // Check again now that we have the lock. Another thread may + // have added extra data entries. + ProfileData* result = bci_to_extra_data_helper(bci, m, dp, false); + if (result != NULL || dp >= end) { return result; } - if (create_if_missing && dp < end) { - assert(dp->tag() == DataLayout::no_tag || (dp->tag() == DataLayout::speculative_trap_data_tag && m != NULL), "should be free"); - assert(next_extra(dp)->tag() == DataLayout::no_tag || next_extra(dp)->tag() == DataLayout::arg_info_data_tag, "should be free or arg info"); - u1 tag = m == NULL ? DataLayout::bit_data_tag : DataLayout::speculative_trap_data_tag; - // SpeculativeTrapData is 2 slots. Make sure we have room. - if (m != NULL && next_extra(dp)->tag() != DataLayout::no_tag) { - return NULL; - } - DataLayout temp; - temp.initialize(tag, bci, 0); - // May have been set concurrently - if (dp->header() != temp.header() && !dp->atomic_set_header(temp.header())) { - // Allocation failure because of concurrent allocation. Try - // again. - continue; - } - assert(dp->tag() == tag, "sane"); - assert(dp->bci() == bci, "no concurrent allocation"); - if (tag == DataLayout::bit_data_tag) { - return new BitData(dp); - } else { - // If being allocated concurrently, one trap may be lost - SpeculativeTrapData* data = new SpeculativeTrapData(dp); - data->set_method(m); - return data; - } + assert(dp->tag() == DataLayout::no_tag || (dp->tag() == DataLayout::speculative_trap_data_tag && m != NULL), "should be free"); + assert(next_extra(dp)->tag() == DataLayout::no_tag || next_extra(dp)->tag() == DataLayout::arg_info_data_tag, "should be free or arg info"); + u1 tag = m == NULL ? DataLayout::bit_data_tag : DataLayout::speculative_trap_data_tag; + // SpeculativeTrapData is 2 slots. Make sure we have room. + if (m != NULL && next_extra(dp)->tag() != DataLayout::no_tag) { + return NULL; + } + DataLayout temp; + temp.initialize(tag, bci, 0); + + dp->set_header(temp.header()); + assert(dp->tag() == tag, "sane"); + assert(dp->bci() == bci, "no concurrent allocation"); + if (tag == DataLayout::bit_data_tag) { + return new BitData(dp); + } else { + SpeculativeTrapData* data = new SpeculativeTrapData(dp); + data->set_method(m); + return data; } - return NULL; } return NULL; } diff --git a/src/share/vm/oops/methodData.hpp b/src/share/vm/oops/methodData.hpp index 2dbcd44c7c91a7999d829bff8642235cb8d10407..ede81b87ee5e817c0f1f120a396c78ac63a9526b 100644 --- a/src/share/vm/oops/methodData.hpp +++ b/src/share/vm/oops/methodData.hpp @@ -190,12 +190,6 @@ public: void set_header(intptr_t value) { _header._bits = value; } - bool atomic_set_header(intptr_t value) { - if (Atomic::cmpxchg_ptr(value, (volatile intptr_t*)&_header._bits, 0) == 0) { - return true; - } - return false; - } intptr_t header() { return _header._bits; } @@ -1856,10 +1850,12 @@ private: // Cached hint for bci_to_dp and bci_to_data int _hint_di; + Mutex _extra_data_lock; + MethodData(methodHandle method, int size, TRAPS); public: static MethodData* allocate(ClassLoaderData* loader_data, methodHandle method, TRAPS); - MethodData() {}; // For ciMethodData + MethodData() : _extra_data_lock(Monitor::leaf, "MDO extra data lock") {}; // For ciMethodData bool is_methodData() const volatile { return true; } @@ -1964,7 +1960,7 @@ private: // What is the index of the first data entry? int first_di() const { return 0; } - ProfileData* bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp); + ProfileData* bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent); // Find or create an extra ProfileData: ProfileData* bci_to_extra_data(int bci, Method* m, bool create_if_missing);