Skip to content

Conversation

@NickCraver
Copy link
Collaborator

In #1854 we switched to a Thread here to prevent the pile-up case, however under concurrent load this incurs a 10-11% penalty given backlogs aren't all that uncommon (due to the lock contention case). In .NET 6.0+, let's use the thread pool growth semantics to handle the stall case and use the cheaper Task.Run() approach.

Running the benchmarks from #1164 I was experimenting with on lock perf, I noticed a large perf sink in thread creation as a result of lock contention. For that benchmark as an example (high concurrent load) we're talking about a ~93 second -> ~84 second run reduction and similar wins in concurrent load tests.

In #1854 we switched to a Thread here to prevent the pile-up case, however under concurrent load this incurs a 10-11% penalty given backlogs aren't all that uncommon (due to the lock contention case). In .NET 6.0+, let's use the thread pool growth semantics to handle the stall case and use the cheaper Task.Run() approach.
@NickCraver NickCraver merged commit c923435 into main Jan 18, 2022
@NickCraver NickCraver deleted the craver/net60-task-backlog branch January 18, 2022 13:34
eerhardt added a commit to eerhardt/StackExchange.Redis that referenced this pull request Jun 29, 2023
This allows for using new APIs introduced in net6.0.

In order to enable building for net6.0, we also need to revert the thread pool changes in StackExchange#1939 and StackExchange#1950. This was already effectively reverted in StackExchange#1992 by not building for net6.0. Now that we are building for net6.0 again, these if-defs need to be removed.
@eerhardt eerhardt mentioned this pull request Jun 29, 2023
NickCraver pushed a commit that referenced this pull request Jul 1, 2023
This allows for using new APIs introduced in net6.0.

In order to enable building for net6.0, we also need to revert the thread pool changes in #1939 and #1950. This was already effectively reverted in #1992 by not building for net6.0. Now that we are building for net6.0 again, these if-defs need to be removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants