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

Roland Westrelin roland.westrelin at oracle.com
Mon Mar 3 04:21:42 PST 2014

Hi Vladimir,

Thanks for reviewing that change.

>> In ciMethodData.cpp: when the ciMethodData is loaded, the code walks over the traps in the extra data to translate their Method into a ciMethod. There can be new traps added as this is happening so the code that walks over the traps should iterate over the ciMethodData copy of the profile data. Because of concurrent updates, the assert is incorrect.
> Load_data() use Copy::disjoint_words() to get snapshot of all data (int total_size = _data_size + _extra_data_size;). Whatever we add after that concurrently should not be taking into account. Can you do that, process only _extra_data_size extra data?

As I understand _extra_data_size takes into account all extra data entries, including the ones that are not yet used and the arg info entries at the end of the MDO. So I’m not sure I understand what you’re proposing.

> I think load_extra_data() should get  extra_data_base(), etc. from ciMethodData copy:
>  81 void ciMethodData::load_extra_data() {
>  82   MethodData* mdo = get_MethodData();
>  83
>  84   // speculative trap entries also hold a pointer to a Method so need to be translated
>  85   DataLayout* dp_src  = mdo->extra_data_base();
>  86   DataLayout* end_src = mdo->extra_data_limit();
>  87   DataLayout* dp_dst  = extra_data_base();

Are you saying that because we make a copy of the MDO we don’t need to read the references to translate from the MDO but we can read them from the copy and then overwrite them?
I followed the pattern that is used elsewhere: read from the MDO the entries that need to be translated.

>> In methodData.cpp: I had to remove the asserts because they are incorrect in case of concurrent updates as well. Also, the test that checks whether there is room for a speculative trap is broken in case of concurrent updates: the intent of next_extra(dp) is to check the next cell but if dp is allocated to a speculative trap concurrently it checks 2 cells from the current cell. Also, next_extra(dp)->tag() != DataLayout::no_tag doesn’t mean there’s no more space because it may have been allocated to some other trap concurrently and there may be more free space after.
> create_if_missing is true only during deoptimization so performance is not important. So can we do update under a lock?
> Concurrency will screw up you in one or an other way if you don't use lock.

That sounds more reasonable. I’ll do that.


> Thanks,
> Vladimir
>> http://cr.openjdk.java.net/~roland/8035841/webrev.00/
>> Roland.

More information about the hotspot-compiler-dev mailing list