RFR(s): 8218147: make_walkable asserts on multiple calls

[Robbin Ehn]

Hi,

> I would say yes, unless you want to add the Atomic::read now.

Sure, full:
http://rehn-ws.se.oracle.com/cr_mirror/8218147/v2/webrev/index.html

Thanks, Robbin

> 
>>>
>>> Any idea what problem the original assert was trying to catch?
>>
>> No... you push it as part of your fix for 8161598 :)
>> I do not see it related, several assert which made sense was added.
>>
> 
> I don't think I took into account concurrent access when I added those asserts :-)
> 
>>>
>>> If it's already set, should we check that _last_Java_pc matches the new value?
>>
>> We manually set the pc in several places, so if it's set, it's not certain that
>> it should be the same as in last sp.
>> I can't distinguish between the cases.
>>
> 
> If we get pc from sp[-1] then it should match, but you're right, we sometimes 
> get pc from somewhere else.
> 
> dl
> 
>> Thanks, Robbin
>>
>>>
>>> dl
>>>
>>> On 4/3/19 3:36 AM, Robbin Ehn wrote:
>>>> Hi all, please review.
>>>>
>>>> If a JavaThread in native both gets selected for java-stack sampling and a
>>>> handshake both VMThread and JFR sampler will call make_walkable. There is an
>>>> assert making sure we do not do this twice. Since we only store _last_Java_pc
>>>> from sp, we can allow it be executed multiple times for both aarch64/x64 
>>>> which have the assert.
>>>>
>>>> The asserts comes from:
>>>> 8161598: Kitchensink fails: assert(nm->insts_contains(original_pc)) failed: 
>>>> original PC must be in nmethod/CompiledMethod
>>>>
>>>> They seems not to be directly connected to the bug.
>>>>
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8218147
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~rehn/8218147/webrev/
>>>>
>>>> Compiled aarch64, x64 passes t1-3.
>>>>
>>>> Thanks, Robbin
>>>
>