John Torjo <john.lists@torjo.com> wrote: <snip>
But now, by looking again at atomic_conditional_increment. I'm not a threading expert, but it seems pretty costly as it is implemented now. Could it not go multiple times (n>=2) thorughout the for?
Yes.
Current implementation:
inline long atomic_conditional_increment(long volatile & value) { for(;;) { long tmp = value; if(tmp == 0) return 0; if(InterlockedCompareExchange(&value, tmp + 1, tmp) == tmp) return tmp + 1; } }
Couldn't it become:
inline long atomic_conditional_increment(long volatile & value) { long tmp = value;
Suppose we read the count as 2 here...
if(tmp == 0) return 0;
...and that another thread decrements the count to 1 around here...
long new_val = InterlockedCompareExchange(&value, tmp + 1, tmp); if(new_val == tmp) return tmp + 1;
...so that InterlockedCompareExchange fails and returns 1, so this doesn't return; and suppose that another thread decrements the count to 0 around here...
else return InterlockedIncrement(&value);
...then this increments the count to 1, which is wrong.
}
The increment has to be conditional on the count being non-zero, and neither the Win32 API nor the x86 processor (AFAIK) can perform that atomically. So the loop is unavoidable. There is room for a slight optimisation in that the value returned by InterlockedCompareExchange can be used the next time round: inline long atomic_conditional_increment(long volatile & value) { long old = value, older; do { if (old == 0) return 0; older = old; old = InterlockedCompareExchange(&value, old, old + 1); } while (old != older); return old + 1; }