<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 2 November 2017 at 04:29, Koos Zevenhoven <span dir="ltr"><<a href="mailto:k7hoven@gmail.com" target="_blank">k7hoven@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div style="font-family:monospace,monospace">SPOILER ALERT! At the moment, Nick's statement is in fact <i>*always*</i> true in <i>*all*</i> cases (at least when ignoring untypical cases and some inaccuracies in phrasing). ​Another question is whether the statement <i>*should*</i> be true at all.​<div style="display:inline">​ </div></div></div><div><span style="font-family:monospace,monospace"><div style="font-family:monospace,monospace;display:inline"><br></div></span></div><div><span style="font-family:monospace,monospace"><div style="font-family:monospace,monospace;display:inline">PyErr_CheckSignals(), the function that checks for pending signals, now <i>*implicitly*</i> uses the strictest possible memory-order requirement (SEQ_CST) for checking the `is_tripped` flag, a value which can be used to peek whether there are any pending signals. This means that two threads that call PyErr_CheckSignals can't even <i>*check*</i> the value of that flag at the same time, and they'll have to wait for each other and whatever the CPU hardware implementation does for cache synchronzation. </div></span></div></div></div></div></blockquote><div><br></div><div>Nice, that's deeper than I went - I just assumed there was an interlocked_increment in there somewhere, without checking whether or not there were any optimised code paths that managed to avoid that call :)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div style="font-family:monospace,monospace">​From a correctness point of view, that is absolutely great: if PyErr_CheckSignals() is called, it is guaranteed to notice a new signal regardles of how small the number of picoseconds after the `is_tripped` flag has been set.  But is that really important? Do we really need things to be slowed down by this? And do we want people to "optimize" code by avoiding signal checking?</div></div></div></div></div></blockquote><div><br></div><div>It isn't signal handling latency that's the concern here, it's potentially missing signals. Consider the case where we have 3 threads: A, B, C. A is the main thread that will actually handle signals, B and C both happened to receive them. We'll also assume  the two threads receive *different* signals (otherwise they'll potentially get collapsed into one notification regardless).</div><div><br></div><div>The scenario we want to avoid is having one or both of the signals set, but is_tripped cleared. With an interlocked check (where both 0->1 and 1->0 are memory barriers), that clearly can't happen. I suspect you're right that this could also be achieved with a less strict memory sync requirement on the 0->1 check, but that's much harder to reason about than simply limiting the number of iterations we make through an iterator consumption loop before checking for signals.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div style="font-family:monospace,monospace">The signals won't be caught until checked anyway, and as we've seen, one solution is to use a counter to determine if enough time has passed that another check for potential signals should happen. That does, however, raise the question of who should own the counter, and if it's OK for multiple threads to use the same counter. If yes, then would we be able to avoid slow atomic decrements (or increments)?</div><div style="font-family:monospace,monospace"><br></div><div style="font-family:monospace,monospace">But another solution might be to make a less strict but almost equivalent functionality with much less overhead. Something like this in a header file:</div><div style="font-family:monospace,monospace"><br></div><div style="font-family:monospace,monospace">static inline int PyErr_PROBE_SIGNALS() {</div><div style="font-family:monospace,monospace">    static int volatile *flag = (int volatile *) &is_tripped;</div><div style="font-family:monospace,monospace">    if (*flag) {</div><div style="font-family:monospace,monospace">        return PyErr_CheckSignals();</div><div style="font-family:monospace,monospace">    }</div><div style="font-family:monospace,monospace">    else {</div><div style="font-family:monospace,monospace">        return 0;</div><div style="font-family:monospace,monospace">    }</div><div style="font-family:monospace,monospace">}</div><br></div><div><div style="font-family:monospace,monospace">​Here, the flag is casted to volatile to force a check to happen each time from memory/cache. However, to make it more standard and to make sure it works with all compilers/platforms, it might be better to, instead of *flag, use an atomic load with "MEMORY_ORDER_RELAXED".​ Another thing is that `is_tripped`, which is currently declared as static inside signalmodule.c [4], should somehow be exposed in the API to make it available for the inline function in headers.</div><br></div><div><div style="font-family:monospace,monospace">​This solution might be fast enough for a lot of cases, although still slightly slower than the counter approach, at least if the counter approach would completely avoid per-iteration memory access by requiring each function to own the counter that is used for this.</div></div></div></div></div></blockquote><div><br></div>One thing I like about a nested inner loop is that it's easy to understand the rationale for it *without* knowing any of the details of how memory synchronisation works, as it's just:</div><div class="gmail_quote"><br></div><div class="gmail_quote">- we want to check for signals regularly so the latency in interrupt handling isn't too high</div><div class="gmail_quote">- PyErr_CheckSignals() is too expensive to call on every iteration</div><div class="gmail_quote">- so we only check every N iterations<br></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">Memory synchronisation then only comes up if someone asks why "PyErr_CheckSignals" is expensive to call. And while it wouldn't surprise at all if you're right and there are ways to make that call cheaper, they're still never going to be cheaper than explicitly reducing the frequency with which it is called.<br clear="all"></div><div class="gmail_extra"><br></div><div class="gmail_extra">Cheers,</div><div class="gmail_extra">Nick.<br></div><div class="gmail_extra"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Nick Coghlan   |   <a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>   |   Brisbane, Australia</div>
</div></div>