Skip to content

Remove QueuePolicy locks #23187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
6 commits merged into from
Jun 30, 2020
Merged

Remove QueuePolicy locks #23187

6 commits merged into from
Jun 30, 2020

Conversation

martincostello
Copy link
Member

This PR removes the two locks from QueuePolicy and replaces them with calls to Interlocked.Increment(ref int)/Interlocked.Decrement(ref int) and changing the property to use a field.

In the case where the request queue is full, the benefit of the removal of the lock for normal operation is offset by the need to call Interlocked.Decrement() to revert the increment.

Also:

  • Fixes a few typos of "availible" to "available".
  • Adds a unit test to verify the Semaphore isn't awaited if the request queue is already full.

Benchmarks

Running the microbenchmarks on my laptop gives the following results (full benchmarks below):

Method Mean Before Mean After Allocs Before Allocs After
WithEmptyQueueOverhead_QueuePolicy(false) 86.603 ns 65.841 ns - -
WithEmptyQueueOverhead_QueuePolicy(true) 1,186.089 ns 1,185.132 ns 280 B 279 B
QueueingAll_QueuePolicy 1.218 μs 1.133 μs 276 B 272 B
RejectingRapidly_QueuePolicy 1.247 μs 1.311 μs 1079 B 1080 B

Before

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-UOQSZQ : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  RunStrategy=Throughput
Method YieldsThreadInternally Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline False 2.954 ns 0.0398 ns 0.0353 ns 338,491,508.2 - - - -
WithEmptyQueueOverhead_QueuePolicy False 86.603 ns 1.6849 ns 2.0057 ns 11,547,005.2 - - - -
WithEmptyQueueOverhead_StackPolicy False 50.556 ns 0.6494 ns 0.6074 ns 19,780,105.0 - - - -
Baseline True 987.124 ns 19.6590 ns 33.3825 ns 1,013,043.7 - - - 112 B
WithEmptyQueueOverhead_QueuePolicy True 1,186.089 ns 13.3438 ns 11.8289 ns 843,107.4 0.0016 - - 280 B
WithEmptyQueueOverhead_StackPolicy True 1,169.924 ns 13.1807 ns 12.3293 ns 854,756.5 0.0016 - - 279 B 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-BIANII : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  InvocationCount=1  
RunStrategy=Throughput  UnrollFactor=1
Method MaxConcurrentRequests Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline 8 1.019 μs 0.0199 μs 0.0278 μs 980,978.1 - - - 120 B
QueueingAll_QueuePolicy 8 1.218 μs 0.0289 μs 0.0837 μs 821,029.0 - - - 276 B
QueueingAll_StackPolicy 8 1.171 μs 0.0280 μs 0.0813 μs 854,134.5 - - - 275 B 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-BIANII : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  InvocationCount=1  
RunStrategy=Throughput  UnrollFactor=1
Method Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline 1.157 μs 0.0260 μs 0.0734 μs 864,172.4 - - - 916 B
RejectingRapidly_QueuePolicy 1.247 μs 0.0351 μs 0.0948 μs 802,155.8 - - - 1079 B
RejectingRapidly_StackPolicy 2.058 μs 0.0410 μs 0.0974 μs 485,829.6 - - - 1098 B

After

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-EDLYXQ : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  RunStrategy=Throughput
Method YieldsThreadInternally Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline False 2.696 ns 0.0271 ns 0.0241 ns 370,952,460.3 - - - -
WithEmptyQueueOverhead_QueuePolicy False 65.841 ns 1.1905 ns 1.1692 ns 15,187,996.9 - - - -
WithEmptyQueueOverhead_StackPolicy False 49.974 ns 0.2900 ns 0.2571 ns 20,010,247.2 - - - -
Baseline True 907.282 ns 7.4458 ns 6.9648 ns 1,102,193.6 - - - 112 B
WithEmptyQueueOverhead_QueuePolicy True 1,185.132 ns 23.5292 ns 29.7569 ns 843,787.6 0.0016 - - 279 B
WithEmptyQueueOverhead_StackPolicy True 1,199.024 ns 22.2960 ns 42.9568 ns 834,011.9 0.0016 - - 279 B 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-IQPHEX : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  InvocationCount=1  
RunStrategy=Throughput  UnrollFactor=1
Method MaxConcurrentRequests Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline 8 1.012 μs 0.0200 μs 0.0371 μs 988,543.2 - - - 120 B
QueueingAll_QueuePolicy 8 1.133 μs 0.0328 μs 0.0956 μs 882,494.5 - - - 272 B
QueueingAll_StackPolicy 8 1.138 μs 0.0226 μs 0.0563 μs 878,815.0 - - - 274 B 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.329 (2004/?/20H1)
Intel Core i9-9980HK CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.6.20266.3
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.31802, CoreFX 5.0.20.31802), X64 RyuJIT
  Job-IQPHEX : .NET Core 5.0.0 (CoreCLR 5.0.20.26401, CoreFX 5.0.20.26401), X64 RyuJIT

Server=True  Toolchain=.NET Core 5.0  InvocationCount=1  
RunStrategy=Throughput  UnrollFactor=1
Method Mean Error StdDev Op/s Gen 0 Gen 1 Gen 2 Allocated
Baseline 1.180 μs 0.0319 μs 0.0921 μs 847,687.7 - - - 916 B
RejectingRapidly_QueuePolicy 1.311 μs 0.0481 μs 0.1380 μs 762,637.8 - - - 1080 B
RejectingRapidly_StackPolicy 2.126 μs 0.0375 μs 0.0792 μs 470,365.7 - - - 1099 B

@martincostello
Use field rather than property
c708dab 
Operate on a field directly, rather than through a property.
@martincostello
Make field readonly
9415776 
Make field read-only as its value is never changed.
@martincostello
Remove lock
7067202 
Use Interlocked.Decrement() instead of taking a lock.
@martincostello
Remove lock for increment
357fc52 
This removes the need for the lock in the success case at the cost of an extra  Interlocked.Decrement() call for the failed case.
@martincostello
Fix typos
37ac33a 
Change "availible" to "available".
@martincostello
Add unit test for full queue
8ed7dd5 
Add a unit test that validates request is not queued if the queue is already full.
@martincostello martincostello requested a review from Tratcher as a code owner June 20, 2020 11:50
@ghost ghost added the area-middleware label Jun 20, 2020
@ghost
Copy link

ghost commented Jun 30, 2020

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@Tratcher Tratcher added this to the 5.0.0-preview8 milestone Jun 30, 2020
@ghost ghost merged commit acabbbc into dotnet:master Jun 30, 2020
@martincostello martincostello deleted the Reduce-QueuePolicy-Locking branch June 30, 2020 06:06
@Tratcher
Copy link
Member

Thanks

@amcasey amcasey added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jun 2, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher Tratcher approved these changes

Assignees

@Tratcher Tratcher

Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares
Projects
None yet
Milestone
5.0.0-preview8
Development

Successfully merging this pull request may close these issues.
3 participants
@martincostello @Tratcher @amcasey