Remove some unnecessary spinning

[stephentoub]

Most of the use of SpinWait in CoreLib involves waiting for some short-lived operation to complete on another thread, in which case the spinning thread should backoff as it's unable to make forward progress until the other operation completes. In a few cases, however, SpinWait is being used just around CompareExchange operations, such that at least one thread running this code path is guaranteed to make forward progress, and the backoff in the spinning doesn't actually help (in theory it could help to reduce contention if lots of threads were all trying to CompareExchange concurrently, but in such cases you'd actually want more randomized backoff, as otherwise it's likely all the threads would re-attempt at around the same time and similarly re-encounter contention).

cc: @kouvel, @jkotas

[jkotas]

Thanks

[kouvel]

Last time I tried removing spinning in some of the CompareExchange loops, it considerably decreased throughput under high levels of contention, I believe it is because the delay removes some contention and allows faster forward progress for some threads. I had used these tests: #13670 (comment). Removing the spin may also decrease latency, so it may be a decent tradeoff?

[stephentoub]

Last time I tried removing spinning in some of the CompareExchange loops, it considerably decreased throughput under high levels of contention

Which loops? Most of the loops that use CompareExchange I haven't touched here, because they don't depend on just a single CompareExchange, but rather they actually need some additional write to be performed in order to make forward progress. For example, ConcurrentQueue.Enqueue/TryDequeue effectively have a spin lock employed across two writes, and so another thread may have to wait for that second write to occur before it can make forward progress. I examined all of the SpinWait usage in coreclr, and these were the only three that appeared to not have such dependencies.

I also don't believe they'd be tested at all by the tests in #13670 (comment)... it's actually really hard to come up with tests that get these code paths to be contended. The cited ConcurrentQueue test will only hit this if it somehow manages to try to enqueue beyond the end of a segment, which will cause it to need to grow into a new segment, which is already an expensive operation: good throughput in concurrent queue is achieved when the queue can reach a steady state where it's operating as a circular buffer solely within a single segment that's large enough to handle the throughput demands of the scenario.

[kouvel]

Makes sense, thanks! I think I tried to remove the spins in TryEnqueue/TryDequeue before. I don't remember if there were other cases without dependencies that also regressed.