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

[Vladimir Ivanov]

John,

Thank you for the detailed explanation.

Best regards,
Vladimir Ivanov

On 12/14/12 2:42 AM, John Rose wrote:
> On Dec 13, 2012, at 4:29 PM, Vladimir Ivanov wrote:
>
>> I'm not very comfortable with memory state management so far, but I
>> don't see how it differs from "fast" fix. Memory graph seems almost
>> the same between these 2 versions. The only difference is that the
>> check of current thread is after the barrier.
>>
>> I'll experiment with converting slow call into uncommon trap and see
>> how it works.
>
> As long as there is a non-removable memory barrier (on the relevant
> memory slice) that dominates the loop backedge, you are good; the loop
> body will not hoist loads (on that memory slice).  Almost any barrier
> position will work, I think, as long as it is unconditional in the loop
> (dominates backedge).
>
> My concern was that (being somehow blinded to your perfectly reasonable
> membar) the only barrier I could see was buried in the slow call, and
> that might go away in a strength-reduced loop, for any of a number of
> reasons.  An uncommon trap would do it, and loop unswitching might do
> it, maybe.  That is not a problem, though, in either of your fixes.
>
>>> > It seems like your "fast" fix is more robust, for this reason.
> I take that back!
>
>>> > 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.
>
>> Good catch! You are right, there's a window (though it's very small)
>> when another thread can interrupt the thread. I didn't like
>> known_current_thread variable and now I have a reason to completely
>> remove it :-)
>
> It might be a reasonable optimization, but if it can fail, even
> infrequently, it should be tested, like this:
>
>    boolean tmp = makeSlowCall(...);
>    if (tmp == false)  uncommon_trap();
>    tmp = true;
>
> It's not clear that the optimization here is worth much, since it only
> kicks in if there has been a slow call, and surely the extra test would
> be noise.  The best benefit is that the optimizer can speculate slightly
> better code after the slow call.  The value of this is unclear.
>
> — John