-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Improve spining in ReaderWriter Lock #13324
Conversation
If you pass an null payload using Write<T>, and in the EventListener then call PayloadNames, it will throw an IndexOutOfRangeException. It should just return null. This fixes this.
Main thing we do is remember how much spinning we did last time and use that to decide how much to do this time. Thus if spinning was unproductive last time we don't spin this time (but will try from time to time to see if things changed).
I did a bunch of tests on variations of this to see what is working and what is not, and to hopefully understand why. I modified my benchmark to add some memory-heavy work on a specified number of threads, these threads just do some simple memory operations (array[i - 1] += array[i]) on an array (64 int elements) and increment a counter for each full iteration over the array. Issues with changes to the spin lock spin heuristics:
Issues with changes to the RW lock spin heuristics:
Effects of and on unrelated memory-intensive work (most of my tests were with at least proc count threads):
I found one thing that sometimes performed an order of magnitude better than deprioritization (but only on top of deprioritization).
In summary:
|
Thanks Kount on doing the performance work to investigate this. I think that the general idea of using history to determine a good spin count is likely to be better than the data above suggests. The reason is that the benchmark we are using above is really a unusual corner case (where reader-writer lock hold time is basically zero, and under super-high contention). In fact the data above supports this because when other work was included you saw that everything performed about the same. Indeed the target for the SpinHistory class was not really to optimize for this case. All I hoped that SpinHistory would do for this super high contention case, is to avoid 'meltdown' (that is we were not become dramatically LESS efficient as contention increased). I expected to quickly cause some threads to block (sleep), and this would dramatically affect that thread's throughput, but at least the threads that were making headway, would have a very bounded amount of spinning (and thus its inefficiency would be bounded). Thus its value is not about optimality (for this case that frankly will never be good and users should avoid), but about simplicity (a good enough solution the never melts down and works well up to a very reasonable scaling factor) SpinHistory actually helps in the case where the lock hold time may vary, but is often too long to spin but short enough that the lock might be taking many time (e.g. say .1 - 10 msec). In that case SpinHistory avoids any spinning (because a reasonable spin time (e.g. 10usec), is almost certainly going to fail and simply waste CPU). Now the myLock should NOT have this hold time characteristic (we expect myLock to only be held < 0.1usec because it is only help during TRANSITIONS for the RW lock). Thus I would not expect it to be particular good, it was just that it avoids meltdown in a simple way. In the end, for reader-writer lock I am OK with prioritizing exits (and writes). It is complexity, but it is not that much and it certainly makes intuitive sense, and solved the meltdown problem. I think SpinHistory is still useful for normal locks (which unlike myLock are more likely to have lock hold times that might be short at times and long at times). But that is a different issue. I will close this one. |
Fixes #12780, I suggested this in words to @kouvel as a alternative to PR #13243. He suggested that I make up the actual code (since the goal was it was supposed to be simple).
Marked NoMerge for now since I have not tested the performance characteristics, and this is mostly so that @kouvel can review.
The change incorporates the optimization in PR #13243 to avoid read spinning if writers are trying to get the lock.
But the main change here is to remember how much we spun last time and use that to determine if spinning is worthwhile this time. Thus if spinning was not fruitful, in the past, we don't spin at all (which is a big improvement).
This is done in both the MyLock and the Reader and Writer spin loops.
The code is basically the SpinHistory class, (which is very straightforward), and then very minor changes to wire it in). The rest is some variable renames, and comments.