8035841: assert(dp_src->tag() == dp_dst->tag()) failed: should be same tags 1 != 0 at ciMethodData.cpp:90

Wed, 05 Mar 2014 09:29:12 +0100

author
roland
date
Wed, 05 Mar 2014 09:29:12 +0100
changeset 6382
1a43981d86ea
parent 6381
8ef3428f54b6
child 6383
f258347cec12

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

src/share/vm/ci/ciMethodData.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/methodData.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/methodData.hpp file | annotate | diff | comparison | revisions
     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  

mercurial