Skip to content

Add named pipes microbenchmark #46472

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.

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

Merged
merged 2 commits into from
Feb 7, 2023
Merged

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Feb 6, 2023

Follow up to #46259. Previous measurements were done in a unit test. BenchmarkDotNet gives a clearer picture of the improvement.

Results:

|    Method | ListenerQueueCount |     Mean |    Error |   StdDev |     Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------- |------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:|
| Plaintext |                  1 | 73.69 us | 1.453 us | 2.765 us | 13,570.0 |     - |     - |     - |     11 KB |
| Plaintext |                  2 | 67.11 us | 1.028 us | 0.962 us | 14,899.9 |     - |     - |     - |     11 KB |
| Plaintext |                  8 | 34.42 us | 0.432 us | 0.361 us | 29,050.4 |     - |     - |     - |     11 KB |
| Plaintext |                 16 | 34.20 us | 0.678 us | 0.857 us | 29,236.3 |     - |     - |     - |     11 KB |

@JamesNK
Copy link
Member Author

JamesNK commented Feb 6, 2023

@davidfowl
image

Results:

|    Method | ListenerQueueCount |     Mean |    Error |   StdDev |     Op/s | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------- |------------------- |---------:|---------:|---------:|---------:|------:|------:|------:|----------:|
| Plaintext |                  1 | 75.40 us | 1.263 us | 1.687 us | 13,262.3 |     - |     - |     - |     11 KB |
| Plaintext |                  2 | 67.21 us | 0.885 us | 0.828 us | 14,878.2 |     - |     - |     - |     11 KB |
| Plaintext |                  8 | 34.52 us | 0.669 us | 0.821 us | 28,966.8 |     - |     - |     - |     11 KB |
| Plaintext |                 16 | 35.37 us | 0.479 us | 0.425 us | 28,275.3 |     - |     - |     - |     11 KB |

Basically the same.

@davidfowl
Copy link
Member

Is it because there's no backpressure? I guess the ConnectionDispatcher always queues to the threadpool immediately after the connection is accepted. When we add connection backpressure this will be a bottleneck.

@JamesNK
Copy link
Member Author

JamesNK commented Feb 6, 2023

One accept loop in ConnectionDispatcher calls NamedPipeConnectionListener.AcceptAsync method. It gets a new connection from NamedPipeConnectionListener.AcceptAsync and immediately puts it on the thread pool. When it calls NamedPipeConnectionListener.AcceptAsync again, another connection is ready for the ConnectionDispatcher to receive immediately.

When we add connection backpressure this will be a bottleneck.

Where is connection backpressure coming from? Who is experiencing the bottleneck? The OS isn't blocking named pipe clients from connecting while this is going on. It has its own accept queue. The purpose of this code is just for the transport to provide connections to the AcceptAsync caller as quickly as possible. Putting things connections on and taking them off the channel isn't a bottleneck.

@davidfowl
Copy link
Member

I mean when we add it. This is a long standing work item that we haven’t don’t yet. The idea is to throttle accepts at the dispatcher level m (probably using the rate limiting APIs).

@JamesNK JamesNK merged commit 20c7ae1 into main Feb 7, 2023
@JamesNK JamesNK deleted the jamesnk/namedpipes-microbenchmark branch February 7, 2023 02:39
@ghost ghost added this to the 8.0-preview2 milestone Feb 7, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants