RFR(m): 8220351: Cross-modifying code

[Reingruber, Richard]

Hi Robin,

  > Hopefully there is no error in my logic :)

At least none that I would have been able to find :)

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

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