RFR(m): 8220351: Cross-modifying code

[Robbin Ehn]

Hi Richard,

On 2019-03-21 10:00, Reingruber, Richard wrote:
> Hi Robin,
> 
>    > Hopefully there is no error in my logic :)
> 
> At least none that I would have been able to find :)

Good :)

> 
> To me the patch looks good. I'm not a reviewer, though.

Thanks!

/Robbin

> 
> Thanks, Richard.
> 
> -----Original Message-----
> From: Robbin Ehn <robbin.ehn at oracle.com>
> Sent: Mittwoch, 20. März 2019 10:32
> To: hotspot-dev at openjdk.java.net; David Holmes <david.holmes at oracle.com>; Doerr, Martin <martin.doerr at sap.com>
> Cc: aph at redhat.com; Erik Österlund <erik.osterlund at oracle.com>; Reingruber, Richard <richard.reingruber at sap.com>
> Subject: Re: RFR(m): 8220351: Cross-modifying code
> 
> Hi, v3 updated with Martin's comments.
> 
> Inc:
> http://cr.openjdk.java.net/~rehn/8220351/v3/inc/
> Full:
> http://cr.openjdk.java.net/~rehn/8220351/v3/
> 
> David, are you okay with this model which does cross-mod fence on leaving a safe
> state?
> With the other model, always cross-mod fence going to java would mean we cannot
> disarm any thread since we don't know which state it's going to to next, if we
> want the same performance.
> 
> Thanks, Robbin
> 
> On 3/19/19 5:43 PM, Robbin Ehn wrote:
>> Hi all, here is v2:
>>
>> Full:
>> http://cr.openjdk.java.net/~rehn/8220351/v2/webrev/
>> Inc:
>> http://cr.openjdk.java.net/~rehn/8220351/v2/inc/webrev/
>>
>> The deprecated non-TLH path is missing the correct cross-mod fence, but since 13
>> will be the last release containing that code path I think that is okay.
>>
>> We just leave native/native trans armed, and let the thread disarm on the
>> transition. If it disarms a new safepoint, we re-arm and stop at next polling
>> site, that avoids recursion.
>>
>> Passes stress and hs-t1-5.
>> As in the previous mail, performance numbers look good.
>>
>> Thanks, Robbin
>>
>> On 3/8/19 4:24 PM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8220351
>>> Changeset:
>>> http://cr.openjdk.java.net/~rehn/8220351/webrev/
>>>
>>> After a JavaThread have been in a safepoint/(handshake) safe state it can start
>>> executing updated code. E.g. an oop in the instruction stream can have been
>>> updated.
>>>
>>> Most hardware's require a barrier or that the code cross modified is far away to
>>> guarantee that the thread executing the updated instruction stream sees the
>>> modification.
>>>
>>> What far away is and how far an update instruction stream is from a safepoint
>>> safe state is not clear.
>>>
>>> To be compliant with those architectures an instruction stream barrier must be
>>> added when leaving the safepoint safe state.
>>>
>>> There may be crashes today due to this missing barrier.
>>> A new CPU with deeper pipeline or changes to the VM which moves a safepoint safe
>>> state closer to a nmethod can increase changes of a crash.
>>>
>>> A few benchmarks will see a performance regression with the extra serializing,
>>> such as Octane-Crypto -5% (x86).
>>>
>>> With a much more elaborate fix, such as changing the local poll to have more
>>> than two states (armed/disarmed), it would be possible to only emit such
>>> instruction stream barrier when the poll have been armed while the thread was
>>> safepoint safe.
>>>
>>> Passes t1-3 on our platforms, and test built what I can.
>>>
>>> Thanks, Robbin