Aaron, Before taking your word for it, I would like to hear more about how your benchmarks relate to smart pointers. It sounds reasonable that the kind of lock described will perform badly in high-contention situations. But what evidence do you have that there is any probability of high contention, especially when, as you say, the locks are short-lived? These locks aren't taken every time the smart pointer is accessed; they're only taken when a copy is made. So to get the high contention you speak of, you would have to have two threads repeatedly making copies of the "same" smart pointer. From where I stand, this seems sufficiently rare that the extra performance gain in the common cases is worth it. /Mattias Quoting "Aaron W. LaFramboise" <aaronrabiddog51@aaronwl.com>:
Tyson Whitehead wrote:
while( InterlockedExchange(&m_.l_, 1) ){ // Note: changed to Sleep(1) from Sleep(0). // According to MSDN, Sleep(0) will never yield // to a lower-priority thread, whereas Sleep(1) // will. Performance seems not to be affected. Sleep(1); }
For what its worth, while it avoids deadlocking problems, this code is also wrong.
Synchronization code should never sleep. That code above will actually sleep for the minimum amount of time Windows allows, which is usually 10ms. I measured the performance of this spinlock a while back when improvements to GCC's Windows mutexes were being discussed on the MinGW lists, and in some cases where there is substanctial contention over short-lived locks, the performance degradation is quite unacceptable. Certain naïve benchmarks might not notice, though, because during the period waiters are sleeping, the lock will be accessable.
I think the code above could lead to severe slowdowns in some usage cases, and could lead to difficult to diagnose intermittant 'blips' of bad performance in others.
In any case, my point is that spinlocks are always going to be wrong in these sorts of situations unless coupled with some other primative. In fact, spinning at all is usually the wrong thing to do. On single CPU systems, it is always a waste of precious time. On multi CPU systems, it can cause very undesirable memory bus slowdowns.
These spinlocks need to be fixed. Is anyone working on this?