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

[Albert Noll]

I guess you ment Roland :-)

Best,
Albert

Von meinem iPhone gesendet

> Am 28.02.2014 um 20:03 schrieb Vladimir Kozlov <vladimir.kozlov at oracle.com>:
> 
> Hi Albert,
> 
>> On 2/28/14 2:47 AM, Roland Westrelin wrote:
>> 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?
> 
> 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();
> 
>> 
>> 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.
> 
> Thanks,
> Vladimir
> 
>> 
>> http://cr.openjdk.java.net/~roland/8035841/webrev.00/
>> 
>> Roland.
>> 
>>