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
--- a/hotspot/src/share/vm/ci/ciMethodData.cpp Tue Mar 04 21:15:33 2014 -0800
+++ b/hotspot/src/share/vm/ci/ciMethodData.cpp Wed Mar 05 09:29:12 2014 +0100
@@ -87,8 +87,9 @@
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 @@
// 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()));
}
}
}
--- a/hotspot/src/share/vm/oops/methodData.cpp Tue Mar 04 21:15:33 2014 -0800
+++ b/hotspot/src/share/vm/oops/methodData.cpp Wed Mar 05 09:29:12 2014 +0100
@@ -1071,7 +1071,8 @@
}
// 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.
@@ -1235,7 +1236,7 @@
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)) {
@@ -1257,10 +1258,11 @@
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;
@@ -1289,40 +1291,40 @@
// 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;
}
- 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;
}
--- a/hotspot/src/share/vm/oops/methodData.hpp Tue Mar 04 21:15:33 2014 -0800
+++ b/hotspot/src/share/vm/oops/methodData.hpp Wed Mar 05 09:29:12 2014 +0100
@@ -190,12 +190,6 @@
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;
}
@@ -2047,10 +2041,12 @@
// 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; }
@@ -2155,7 +2151,7 @@
// 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);