Skip to content

Commit

Permalink
Rollback: remove .NET 6 build (#1992)
Browse files Browse the repository at this point in the history
This removes the .NET 6 specific build from the main library. In testing many previous performance issues filed around past releases (vs. 1.2.6), I was testing this example:
```cs
using StackExchange.Redis;
using System;
using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;

var options = ConfigurationOptions.Parse("localhost,abortConnect=false");
using ConnectionMultiplexer muxer = ConnectionMultiplexer.Connect(options);
var subscriber = muxer.GetSubscriber();
int count = 0;
var sw = Stopwatch.StartNew();
Parallel.For(0, 1000, x =>
{
	subscriber.Publish("cache-events:cache-testing", "hey");
	Interlocked.Increment(ref count);
});
sw.Stop();
Console.WriteLine($"Total in {sw.ElapsedMilliseconds} ms");
```

This parallel thread contention state is one of the things we hoped the .NET 6 behavior for thread pools would help with. It comes from 2 main factors: defaulting to the primary thread pool (rather than our own) and the backlog moving back to `Task.Run` instead of a thread (which has ~11% impact on throughput due to startup cost). This happens here:
- [Socket Manager -> ThreadPool](https://github.com/StackExchange/StackExchange.Redis/blob/74adca8d69ced316b2d4f57d2e8375d40de06335/src/StackExchange.Redis/ConnectionMultiplexer.ReaderWriter.cs#L29-L33)
- [PhysicalBridge backlog](https://github.com/StackExchange/StackExchange.Redis/blob/74adca8d69ced316b2d4f57d2e8375d40de06335/src/StackExchange.Redis/PhysicalBridge.cs#L821-L824)

In the `net5.0` build the sample takes ~100-105ms on an 8-core machine, running under .NET 6. With the `net6.0` build in play (before this PR) and using those changes, we end up in the 20,000-30,000ms territory. With only the socket manager change (leaving the Backlog as a `Task.Run`), we're in the 400-500ms territory.

In short, trying to use the .NET 6 changes to our advantage here greatly regresses some use cases drastically enough I advise we do not enable it. We should look at these cases further before adding the `net6.0` paths to any release build. I can go further than this PR and remove the code paths completely, but I'd like to have these in play easily to discuss with the .NET team as a use case to look at and get some advice.

cc @stephentoub on this one (happy to sync up and help repro)
  • Loading branch information
NickCraver committed Feb 15, 2022
1 parent 4fb787c commit cbc7cc9
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/StackExchange.Redis/StackExchange.Redis.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<!-- extend the default lib targets for the main lib; mostly because of "vectors" -->
<TargetFrameworks>net461;netstandard2.0;net472;netcoreapp3.1;net5.0;net6.0</TargetFrameworks>
<!-- Note: we're NOT building for .NET 6 because the thread pool changes are not wins yet -->
<TargetFrameworks>net461;netstandard2.0;net472;netcoreapp3.1;net5.0</TargetFrameworks>
<Description>High performance Redis client, incorporating both synchronous and asynchronous usage.</Description>
<AssemblyName>StackExchange.Redis</AssemblyName>
<AssemblyTitle>StackExchange.Redis</AssemblyTitle>
Expand Down

0 comments on commit cbc7cc9

Please sign in to comment.