RFR (S): 8003135: HotSpot inlines and hoists the Thread.currentThread().isInterrupted() out of the loop

[John Rose]

On Dec 13, 2012, at 3:02 PM, Vladimir Ivanov wrote:

> http://cr.openjdk.java.net/~vlivanov/8003135/simple/webrev.00
> 31 lines changed: 16 ins; 6 del; 9 mod
> 
> In the IR produced by Thread.isInterrupted(Z)Z intrinsic it's possible to hoist the load of _interrupted flag out of the loop. The fix is to add a barrier to forbid such optimization.
> 
> Proposed fix is cleaner that [1] (no explicit memory state management).
> The place for the barrier is deliberately chosen earlier than necessary and it blocks hoisting of some loop invariants when Thread.isInterrupted() is called in a loop. But I think it's a fair price since I don't think that Thread.isInterrupted(Z)Z performance in tight loops is critical.

I don't see any new barrier in the simple fix.  Where is it?  It looks like the load of thr._interrupted can be hoisted under the same circumstances as before.  If the slow path is somehow turned into a loop exit (via uncommon trap, maybe), then the memory state will fold up and the bit load can be hoisted.

It seems like your "fast" fix is more robust, for this reason.

Also, it's not your bug, but I'm not convinced that "slow_val = intcon(1)" is correct.  I think another thread could come in and clear the interrupted bit, and this call to the native function could return false.

> If you prefer [1] let me know.
> 
> Also, cleaned up the code around a little.

Good cleanups.

— John

> Testing: failing test, manual (verified generated code), JPRT (in progress)
> 
> Thanks!
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] http://cr.openjdk.java.net/~vlivanov/8003135/fast/webrev.00