Wed, 05 Mar 2014 09:29:12 +0100
8035841: assert(dp_src->tag() == dp_dst->tag()) failed: should be same tags 1 != 0 at ciMethodData.cpp:90
Summary: concurrent update of traps with construction of ciMethodData
Reviewed-by: kvn, twisti
1.1 --- a/src/share/vm/ci/ciMethodData.cpp Mon Mar 17 11:54:14 2014 -0700 1.2 +++ b/src/share/vm/ci/ciMethodData.cpp Wed Mar 05 09:29:12 2014 +0100 1.3 @@ -87,8 +87,9 @@ 1.4 DataLayout* dp_dst = extra_data_base(); 1.5 for (;; dp_src = MethodData::next_extra(dp_src), dp_dst = MethodData::next_extra(dp_dst)) { 1.6 assert(dp_src < end_src, "moved past end of extra data"); 1.7 - assert(dp_src->tag() == dp_dst->tag(), err_msg("should be same tags %d != %d", dp_src->tag(), dp_dst->tag())); 1.8 - switch(dp_src->tag()) { 1.9 + // New traps in the MDO can be added as we translate the copy so 1.10 + // look at the entries in the copy. 1.11 + switch(dp_dst->tag()) { 1.12 case DataLayout::speculative_trap_data_tag: { 1.13 ciSpeculativeTrapData* data_dst = new ciSpeculativeTrapData(dp_dst); 1.14 SpeculativeTrapData* data_src = new SpeculativeTrapData(dp_src); 1.15 @@ -102,7 +103,7 @@ 1.16 // An empty slot or ArgInfoData entry marks the end of the trap data 1.17 return; 1.18 default: 1.19 - fatal(err_msg("bad tag = %d", dp_src->tag())); 1.20 + fatal(err_msg("bad tag = %d", dp_dst->tag())); 1.21 } 1.22 } 1.23 }
2.1 --- a/src/share/vm/oops/methodData.cpp Mon Mar 17 11:54:14 2014 -0700 2.2 +++ b/src/share/vm/oops/methodData.cpp Wed Mar 05 09:29:12 2014 +0100 2.3 @@ -1066,7 +1066,8 @@ 2.4 } 2.5 2.6 // Initialize the MethodData* corresponding to a given method. 2.7 -MethodData::MethodData(methodHandle method, int size, TRAPS) { 2.8 +MethodData::MethodData(methodHandle method, int size, TRAPS) 2.9 + : _extra_data_lock(Monitor::leaf, "MDO extra data lock") { 2.10 No_Safepoint_Verifier no_safepoint; // init function atomic wrt GC 2.11 ResourceMark rm; 2.12 // Set the method back-pointer. 2.13 @@ -1230,7 +1231,7 @@ 2.14 return (DataLayout*)((address)dp + DataLayout::compute_size_in_bytes(nb_cells)); 2.15 } 2.16 2.17 -ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp) { 2.18 +ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent) { 2.19 DataLayout* end = extra_data_limit(); 2.20 2.21 for (;; dp = next_extra(dp)) { 2.22 @@ -1252,10 +1253,11 @@ 2.23 if (m != NULL) { 2.24 SpeculativeTrapData* data = new SpeculativeTrapData(dp); 2.25 // data->method() may be null in case of a concurrent 2.26 - // allocation. Assume it's for the same method and use that 2.27 + // allocation. Maybe it's for the same method. Try to use that 2.28 // entry in that case. 2.29 if (dp->bci() == bci) { 2.30 if (data->method() == NULL) { 2.31 + assert(concurrent, "impossible because no concurrent allocation"); 2.32 return NULL; 2.33 } else if (data->method() == m) { 2.34 return data; 2.35 @@ -1284,40 +1286,40 @@ 2.36 // Allocation in the extra data space has to be atomic because not 2.37 // all entries have the same size and non atomic concurrent 2.38 // allocation would result in a corrupted extra data space. 2.39 - while (true) { 2.40 - ProfileData* result = bci_to_extra_data_helper(bci, m, dp); 2.41 - if (result != NULL) { 2.42 + ProfileData* result = bci_to_extra_data_helper(bci, m, dp, true); 2.43 + if (result != NULL) { 2.44 + return result; 2.45 + } 2.46 + 2.47 + if (create_if_missing && dp < end) { 2.48 + MutexLocker ml(&_extra_data_lock); 2.49 + // Check again now that we have the lock. Another thread may 2.50 + // have added extra data entries. 2.51 + ProfileData* result = bci_to_extra_data_helper(bci, m, dp, false); 2.52 + if (result != NULL || dp >= end) { 2.53 return result; 2.54 } 2.55 2.56 - if (create_if_missing && dp < end) { 2.57 - assert(dp->tag() == DataLayout::no_tag || (dp->tag() == DataLayout::speculative_trap_data_tag && m != NULL), "should be free"); 2.58 - assert(next_extra(dp)->tag() == DataLayout::no_tag || next_extra(dp)->tag() == DataLayout::arg_info_data_tag, "should be free or arg info"); 2.59 - u1 tag = m == NULL ? DataLayout::bit_data_tag : DataLayout::speculative_trap_data_tag; 2.60 - // SpeculativeTrapData is 2 slots. Make sure we have room. 2.61 - if (m != NULL && next_extra(dp)->tag() != DataLayout::no_tag) { 2.62 - return NULL; 2.63 - } 2.64 - DataLayout temp; 2.65 - temp.initialize(tag, bci, 0); 2.66 - // May have been set concurrently 2.67 - if (dp->header() != temp.header() && !dp->atomic_set_header(temp.header())) { 2.68 - // Allocation failure because of concurrent allocation. Try 2.69 - // again. 2.70 - continue; 2.71 - } 2.72 - assert(dp->tag() == tag, "sane"); 2.73 - assert(dp->bci() == bci, "no concurrent allocation"); 2.74 - if (tag == DataLayout::bit_data_tag) { 2.75 - return new BitData(dp); 2.76 - } else { 2.77 - // If being allocated concurrently, one trap may be lost 2.78 - SpeculativeTrapData* data = new SpeculativeTrapData(dp); 2.79 - data->set_method(m); 2.80 - return data; 2.81 - } 2.82 + assert(dp->tag() == DataLayout::no_tag || (dp->tag() == DataLayout::speculative_trap_data_tag && m != NULL), "should be free"); 2.83 + assert(next_extra(dp)->tag() == DataLayout::no_tag || next_extra(dp)->tag() == DataLayout::arg_info_data_tag, "should be free or arg info"); 2.84 + u1 tag = m == NULL ? DataLayout::bit_data_tag : DataLayout::speculative_trap_data_tag; 2.85 + // SpeculativeTrapData is 2 slots. Make sure we have room. 2.86 + if (m != NULL && next_extra(dp)->tag() != DataLayout::no_tag) { 2.87 + return NULL; 2.88 } 2.89 - return NULL; 2.90 + DataLayout temp; 2.91 + temp.initialize(tag, bci, 0); 2.92 + 2.93 + dp->set_header(temp.header()); 2.94 + assert(dp->tag() == tag, "sane"); 2.95 + assert(dp->bci() == bci, "no concurrent allocation"); 2.96 + if (tag == DataLayout::bit_data_tag) { 2.97 + return new BitData(dp); 2.98 + } else { 2.99 + SpeculativeTrapData* data = new SpeculativeTrapData(dp); 2.100 + data->set_method(m); 2.101 + return data; 2.102 + } 2.103 } 2.104 return NULL; 2.105 }
3.1 --- a/src/share/vm/oops/methodData.hpp Mon Mar 17 11:54:14 2014 -0700 3.2 +++ b/src/share/vm/oops/methodData.hpp Wed Mar 05 09:29:12 2014 +0100 3.3 @@ -190,12 +190,6 @@ 3.4 void set_header(intptr_t value) { 3.5 _header._bits = value; 3.6 } 3.7 - bool atomic_set_header(intptr_t value) { 3.8 - if (Atomic::cmpxchg_ptr(value, (volatile intptr_t*)&_header._bits, 0) == 0) { 3.9 - return true; 3.10 - } 3.11 - return false; 3.12 - } 3.13 intptr_t header() { 3.14 return _header._bits; 3.15 } 3.16 @@ -1856,10 +1850,12 @@ 3.17 // Cached hint for bci_to_dp and bci_to_data 3.18 int _hint_di; 3.19 3.20 + Mutex _extra_data_lock; 3.21 + 3.22 MethodData(methodHandle method, int size, TRAPS); 3.23 public: 3.24 static MethodData* allocate(ClassLoaderData* loader_data, methodHandle method, TRAPS); 3.25 - MethodData() {}; // For ciMethodData 3.26 + MethodData() : _extra_data_lock(Monitor::leaf, "MDO extra data lock") {}; // For ciMethodData 3.27 3.28 bool is_methodData() const volatile { return true; } 3.29 3.30 @@ -1964,7 +1960,7 @@ 3.31 // What is the index of the first data entry? 3.32 int first_di() const { return 0; } 3.33 3.34 - ProfileData* bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp); 3.35 + ProfileData* bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent); 3.36 // Find or create an extra ProfileData: 3.37 ProfileData* bci_to_extra_data(int bci, Method* m, bool create_if_missing); 3.38