Add inline scheduler option for Sockets transport#24638
Conversation
| /// <remarks> | ||
| /// This will run application code on the IO thread which is why this is unsafe. | ||
| /// </remarks> | ||
| public bool UnsafeInlineScheduling { get; set; } |
There was a problem hiding this comment.
Doesn't sound unsafe enough!
There was a problem hiding this comment.
| public bool UnsafeInlineScheduling { get; set; } | |
| public bool UnsafePreferInlineScheduling { get; set; } |
There was a problem hiding this comment.
- We add the API and get the API added in dotnet/runtime. Setting our API should set both.
- We add the API and We use private reflection to set it on the socket itself. Setting our API should set both.
- We don't expose a public API and we also consume the env variable. (and we have an opt-out for weird cases)
There was a problem hiding this comment.
Looks like the env variable in Runtime globally affects some settings and even if you use private reflection to set the per Socket setting you don't affect the other things the env variable does. This results in massive perf regressions, i.e. 100k RPS without the env variable but with private reflection vs. 1.2M RPS with the env variable and no private reflection.
|
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
| /// Inline application and transport continuations instead of dispatching to the threadpool. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This will run application code on the IO thread which is why this is unsafe. |
There was a problem hiding this comment.
Explain how this can hurt your performance. Also document the DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS environment variable.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
It's great that this feature made it to 5.0! 👍 |
Fixes #20952