8057038: Speculative traps not robust when compilation and class unloading are concurrent
Summary: speculative traps can be removed from MDO while being copied by compiler
Reviewed-by: kvn, iveresov
--- a/hotspot/src/share/vm/ci/ciMethodData.cpp Wed Sep 03 15:29:57 2014 +0400
+++ b/hotspot/src/share/vm/ci/ciMethodData.cpp Sat Aug 02 06:15:23 2014 +0200
@@ -81,19 +81,38 @@
void ciMethodData::load_extra_data() {
MethodData* mdo = get_MethodData();
+ MutexLocker(mdo->extra_data_lock());
+
// speculative trap entries also hold a pointer to a Method so need to be translated
DataLayout* dp_src = mdo->extra_data_base();
- DataLayout* end_src = mdo->extra_data_limit();
+ DataLayout* end_src = mdo->args_data_limit();
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");
- // 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()) {
+ assert(((intptr_t)dp_dst) - ((intptr_t)extra_data_base()) == ((intptr_t)dp_src) - ((intptr_t)mdo->extra_data_base()), "source and destination don't match");
+
+ // New traps in the MDO may have been added since we copied the
+ // data (concurrent deoptimizations before we acquired
+ // extra_data_lock above) or can be removed (a safepoint may occur
+ // in the translate_from call below) as we translate the copy:
+ // update the copy as we go.
+ int tag = dp_src->tag();
+ if (tag != DataLayout::arg_info_data_tag) {
+ memcpy(dp_dst, dp_src, ((intptr_t)MethodData::next_extra(dp_src)) - ((intptr_t)dp_src));
+ }
+
+ switch(tag) {
case DataLayout::speculative_trap_data_tag: {
ciSpeculativeTrapData* data_dst = new ciSpeculativeTrapData(dp_dst);
SpeculativeTrapData* data_src = new SpeculativeTrapData(dp_src);
+
data_dst->translate_from(data_src);
+
+#ifdef ASSERT
+ SpeculativeTrapData* data_src2 = new SpeculativeTrapData(dp_src);
+ assert(data_src2->method() == data_src->method() && data_src2->bci() == data_src->bci(), "entries changed while translating");
+#endif
+
break;
}
case DataLayout::bit_data_tag:
@@ -244,8 +263,8 @@
}
ciProfileData* ciMethodData::bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots) {
- DataLayout* dp = data_layout_at(data_size());
- DataLayout* end = data_layout_at(data_size() + extra_data_size());
+ DataLayout* dp = extra_data_base();
+ DataLayout* end = args_data_limit();
two_free_slots = false;
for (;dp < end; dp = MethodData::next_extra(dp)) {
switch(dp->tag()) {
@@ -492,8 +511,8 @@
ciArgInfoData *ciMethodData::arg_info() const {
// Should be last, have to skip all traps.
- DataLayout* dp = data_layout_at(data_size());
- DataLayout* end = data_layout_at(data_size() + extra_data_size());
+ DataLayout* dp = extra_data_base();
+ DataLayout* end = args_data_limit();
for (; dp < end; dp = MethodData::next_extra(dp)) {
if (dp->tag() == DataLayout::arg_info_data_tag)
return new ciArgInfoData(dp);
@@ -535,8 +554,8 @@
}
void ciMethodData::dump_replay_data_extra_data_helper(outputStream* out, int round, int& count) {
- DataLayout* dp = data_layout_at(data_size());
- DataLayout* end = data_layout_at(data_size() + extra_data_size());
+ DataLayout* dp = extra_data_base();
+ DataLayout* end = args_data_limit();
for (;dp < end; dp = MethodData::next_extra(dp)) {
switch(dp->tag()) {
@@ -653,8 +672,8 @@
data->print_data_on(st);
}
st->print_cr("--- Extra data:");
- DataLayout* dp = data_layout_at(data_size());
- DataLayout* end = data_layout_at(data_size() + extra_data_size());
+ DataLayout* dp = extra_data_base();
+ DataLayout* end = args_data_limit();
for (;; dp = MethodData::next_extra(dp)) {
assert(dp < end, "moved past end of extra data");
switch (dp->tag()) {
--- a/hotspot/src/share/vm/ci/ciMethodData.hpp Wed Sep 03 15:29:57 2014 +0400
+++ b/hotspot/src/share/vm/ci/ciMethodData.hpp Sat Aug 02 06:15:23 2014 +0200
@@ -410,6 +410,9 @@
// Area dedicated to parameters. NULL if no parameter profiling for
// this method.
DataLayout* _parameters;
+ int parameters_size() const {
+ return _parameters == NULL ? 0 : parameters_type_data()->size_in_bytes();
+ }
ciMethodData(MethodData* md);
ciMethodData();
@@ -461,9 +464,6 @@
address data_base() const {
return (address) _data;
}
- DataLayout* limit_data_position() const {
- return (DataLayout*)((address)data_base() + _data_size);
- }
void load_extra_data();
ciProfileData* bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots);
@@ -524,7 +524,9 @@
ciProfileData* next_data(ciProfileData* current);
bool is_valid(ciProfileData* current) { return current != NULL; }
- DataLayout* extra_data_base() const { return limit_data_position(); }
+ DataLayout* extra_data_base() const { return data_layout_at(data_size()); }
+ DataLayout* args_data_limit() const { return data_layout_at(data_size() + extra_data_size() -
+ parameters_size()); }
// Get the data at an arbitrary bci, or NULL if there is none. If m
// is not NULL look for a SpeculativeTrapData if any first.
--- a/hotspot/src/share/vm/oops/methodData.cpp Wed Sep 03 15:29:57 2014 +0400
+++ b/hotspot/src/share/vm/oops/methodData.cpp Sat Aug 02 06:15:23 2014 +0200
@@ -86,7 +86,7 @@
char* ProfileData::print_data_on_helper(const MethodData* md) const {
DataLayout* dp = md->extra_data_base();
- DataLayout* end = md->extra_data_limit();
+ DataLayout* end = md->args_data_limit();
stringStream ss;
for (;; dp = MethodData::next_extra(dp)) {
assert(dp < end, "moved past end of extra data");
@@ -1048,14 +1048,15 @@
stream->next();
data->post_initialize(stream, this);
}
- if (_parameters_type_data_di != -1) {
+ if (_parameters_type_data_di != no_parameters) {
parameters_type_data()->post_initialize(NULL, this);
}
}
// Initialize the MethodData* corresponding to a given method.
MethodData::MethodData(methodHandle method, int size, TRAPS)
- : _extra_data_lock(Monitor::leaf, "MDO extra data lock") {
+ : _extra_data_lock(Monitor::leaf, "MDO extra data lock"),
+ _parameters_type_data_di(parameters_uninitialized) {
No_Safepoint_Verifier no_safepoint; // init function atomic wrt GC
ResourceMark rm;
// Set the method back-pointer.
@@ -1111,7 +1112,7 @@
DataLayout *dp = data_layout_at(data_size + extra_size + arg_data_size);
dp->initialize(DataLayout::parameters_type_data_tag, 0, parms_cell);
} else {
- _parameters_type_data_di = -1;
+ _parameters_type_data_di = no_parameters;
}
// Set an initial hint. Don't use set_hint_di() because
@@ -1236,7 +1237,7 @@
}
ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent) {
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
for (;; dp = next_extra(dp)) {
assert(dp < end, "moved past end of extra data");
@@ -1285,7 +1286,7 @@
"code needs to be adjusted");
DataLayout* dp = extra_data_base();
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
// Allocation in the extra data space has to be atomic because not
// all entries have the same size and non atomic concurrent
@@ -1330,7 +1331,7 @@
ArgInfoData *MethodData::arg_info() {
DataLayout* dp = extra_data_base();
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
for (; dp < end; dp = next_extra(dp)) {
if (dp->tag() == DataLayout::arg_info_data_tag)
return new ArgInfoData(dp);
@@ -1357,7 +1358,7 @@
void MethodData::print_data_on(outputStream* st) const {
ResourceMark rm;
ProfileData* data = first_data();
- if (_parameters_type_data_di != -1) {
+ if (_parameters_type_data_di != no_parameters) {
parameters_type_data()->print_data_on(st);
}
for ( ; is_valid(data); data = next_data(data)) {
@@ -1367,7 +1368,7 @@
}
st->print_cr("--- Extra data:");
DataLayout* dp = extra_data_base();
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
for (;; dp = next_extra(dp)) {
assert(dp < end, "moved past end of extra data");
// No need for "OrderAccess::load_acquire" ops,
@@ -1565,7 +1566,7 @@
// redefined method
void MethodData::clean_extra_data(CleanExtraDataClosure* cl) {
DataLayout* dp = extra_data_base();
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
int shift = 0;
for (; dp < end; dp = next_extra(dp)) {
@@ -1610,7 +1611,7 @@
void MethodData::verify_extra_data_clean(CleanExtraDataClosure* cl) {
#ifdef ASSERT
DataLayout* dp = extra_data_base();
- DataLayout* end = extra_data_limit();
+ DataLayout* end = args_data_limit();
for (; dp < end; dp = next_extra(dp)) {
switch(dp->tag()) {
--- a/hotspot/src/share/vm/oops/methodData.hpp Wed Sep 03 15:29:57 2014 +0400
+++ b/hotspot/src/share/vm/oops/methodData.hpp Sat Aug 02 06:15:23 2014 +0200
@@ -2107,7 +2107,12 @@
// data index for the area dedicated to parameters. -1 if no
// parameter profiling.
+ enum { no_parameters = -2, parameters_uninitialized = -1 };
int _parameters_type_data_di;
+ int parameters_size_in_bytes() const {
+ ParametersTypeData* param = parameters_type_data();
+ return param == NULL ? 0 : param->size_in_bytes();
+ }
// Beginning of the data entries
intptr_t _data[1];
@@ -2130,7 +2135,7 @@
// Helper for data_at
DataLayout* limit_data_position() const {
- return (DataLayout*)((address)data_base() + _data_size);
+ return data_layout_at(_data_size);
}
bool out_of_bounds(int data_index) const {
return data_index >= data_size();
@@ -2371,10 +2376,11 @@
}
// Add a handful of extra data records, for trap tracking.
- DataLayout* extra_data_base() const { return limit_data_position(); }
+ DataLayout* extra_data_base() const { return limit_data_position(); }
DataLayout* extra_data_limit() const { return (DataLayout*)((address)this + size_in_bytes()); }
- int extra_data_size() const { return (address)extra_data_limit()
- - (address)extra_data_base(); }
+ DataLayout* args_data_limit() const { return (DataLayout*)((address)this + size_in_bytes() -
+ parameters_size_in_bytes()); }
+ int extra_data_size() const { return (address)extra_data_limit() - (address)extra_data_base(); }
static DataLayout* next_extra(DataLayout* dp);
// Return (uint)-1 for overflow.
@@ -2429,11 +2435,12 @@
// Return pointer to area dedicated to parameters in MDO
ParametersTypeData* parameters_type_data() const {
- return _parameters_type_data_di != -1 ? data_layout_at(_parameters_type_data_di)->data_in()->as_ParametersTypeData() : NULL;
+ assert(_parameters_type_data_di != parameters_uninitialized, "called too early");
+ return _parameters_type_data_di != no_parameters ? data_layout_at(_parameters_type_data_di)->data_in()->as_ParametersTypeData() : NULL;
}
int parameters_type_data_di() const {
- assert(_parameters_type_data_di != -1, "no args type data");
+ assert(_parameters_type_data_di != parameters_uninitialized && _parameters_type_data_di != no_parameters, "no args type data");
return _parameters_type_data_di;
}
@@ -2480,8 +2487,8 @@
static bool profile_return_jsr292_only();
void clean_method_data(BoolObjectClosure* is_alive);
-
void clean_weak_method_links();
+ Mutex* extra_data_lock() { return &_extra_data_lock; }
};
#endif // SHARE_VM_OOPS_METHODDATAOOP_HPP