Skip to content
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

DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS and UnsafePreferInlineScheduling Support #59157

Open
Rodrigo-Andrade opened this issue Sep 15, 2021 · 8 comments
Labels
area-System.Net.Sockets enhancement Product code improvement that does NOT require public API changes/additions tenet-performance Performance related issue
Milestone

Comments

@Rodrigo-Andrade
Copy link

I have being using this with great results for more than a year now, any plans for making this more "official"?

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 15, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@stephentoub
Copy link
Member

I have being using this with great results

Can you share more details?

@ghost
Copy link

ghost commented Sep 15, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

I have being using this with great results for more than a year now, any plans for making this more "official"?

Author: Rodrigo-Andrade
Assignees: -
Labels:

area-System.Net.Sockets, untriaged

Milestone: -

@Rodrigo-Andrade
Copy link
Author

Can you share more details?

I talked a bit about that here:
#44449

But long story short: since the change of how sockets queue work in linux, my app had some performance regressions.
After enabling both flags, app performance doubled by avoiding queuing work in the threadpool.

@halter73
Copy link
Member

At least for UnsafePreferInlineScheduling in Kestrel, the concern is this could cause worse performance and possibly even deadlocks given blocking code. I assume there are similar concerns for DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS.

Kestrel defaults to AllowSynchronousIO being false, but that just prevents calls to HttpContext.Request.Body.Read(...) and not HttpContext.Request.Body.ReadAsync(...).Result and other similar patterns. Even blocking on something like a database transaction or an outgoing HTTP request from a request handler could be a big problem if Kestrel's Socket transport doesn't dispatch to the thread pool.

@Rodrigo-Andrade
Copy link
Author

Yes, and i had this problem (deadlocks), that actually led to improvements as some code paths were silently blocking.
But i am not proposing it to be the default, just that it is a legitimate option, right now it's my understanding that it's just experimental and can be pulled at any moment.

@karelz karelz added this to the Future milestone Oct 19, 2021
@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions tenet-performance Performance related issue and removed untriaged New issue has not been triaged by the area owner labels Oct 19, 2021
@karelz
Copy link
Member

karelz commented Oct 19, 2021

Triage: Currently we do not have plans to productize it, but we want to do more investigation in 7.0 + collect feedback from users who use, so we will keep it open.

@ShreyasJejurkar
Copy link
Contributor

Please consider making this option as supported instead of experiment if not on by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Sockets enhancement Product code improvement that does NOT require public API changes/additions tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

5 participants