v2.5 Backlog: Use AutoResetEvent for backlog thread lingering

[NickCraver]

This prevents so many threads from starting/stopping as we finish flushing a backlog. In short: starting a Thread is expensive, really expensive in the grand scheme of things. By ending the thread immediately when a backlog finished flushing, it had a decent change to start back up immediately to get the next item if a backlog was triggered by the lock transfer.

The act of finishing the backlog itself was happening inside the lock and exiting could take a moment causing an immediate re-queue of a follow-up item. This meant: lots of threads starting under parallel high contention load leading to higher CPU, more thread starts, and more allocations from thread starts.

Here's a memory view:

Before	After

(note the 570k object[] instances - those are almost entirely thread starts)

Here are thread views:

Before	After

(note the scrollbar size)

This was initially discovered from testing some of the heavy parallel scenarios vs 1.2.6. For an example scenario with heavy ops load (pictured in profiles above):

Run	1.2.6	2.2.88	main (v2.5.x)	After PR
1	82,426 ms	91,135 ms	99,262 ms	84,562 ms
2	82,335 ms	90,674 ms	100,462 ms	86,211 ms
3	82,059 ms	91,041 ms	99,968 ms	86,283 ms
4	82,435 ms	90,645 ms	102,968 ms	86,153 ms

So note that this is an improvement over 2.2.88 even with the new backlog functionality. We're not quite at the 1.2.6 levels of performance but we're a) closer, and b) a lot of things are more correct in 2.x, and there's a cost to that. I'm very happy with the wins here.

All of that is timings, but CPU usage for the same load is dramatically lower as well though this will depend on workload.

Example code:

using StackExchange.Redis;
using System.Diagnostics;

Stopwatch stopwatch = new Stopwatch();
stopwatch.Start();

var taskList = new List<Task>();
var options = ConfigurationOptions.Parse("localhost");
var connection = ConnectionMultiplexer.Connect(options);
for (int i = 0; i < 10; i++)
{
	var i1 = i;
	var task = new Task(() => Run(i1, connection));
	task.Start();
	taskList.Add(task);
}

Task.WaitAll(taskList.ToArray());
stopwatch.Stop();
Console.WriteLine($"Done. {stopwatch.ElapsedMilliseconds} ms");

static void Run(int taskId, ConnectionMultiplexer connection)
{
	Console.WriteLine($"{taskId} Started");
	var database = connection.GetDatabase(0);

	for (int i = 0; i < 100000; i++)
	{
		database.StringSet(i.ToString(), i.ToString());
	}

	Console.WriteLine($"{taskId} Insert completed");

	for (int i = 0; i < 100000; i++)
	{
		var result = database.StringGet(i.ToString());
	}
	Console.WriteLine($"{taskId} Completed");
}

Anyway...yeah, this was a problem. An AutoReset event is the best way I can think of to solve it. Throwing this up for review, maybe we have an even better idea of how to solve it.

[philon-msft]

Use .ForAwait() instead of .ConfigureAwait(false)?

[philon-msft]

The new backlog code added quite a few other awaited calls without .ForAwait(). Do those need to be updated too?

[NickCraver]

This isn't actually relevant either way - I was testing an allocation thing - can be removed entirely (we kick it off without a context in a thread with none). Can tidy as a follow-up though!

[kevin-montrose]

I don't see where this is disposed?