Skip to content
This repository was archived by the owner on May 16, 2025. It is now read-only.

Add SocketEngineType.DefaultTransport#69

Merged
antonfirsov merged 2 commits into
tmds:masterfrom
antonfirsov:af/default-sockets
Mar 21, 2020
Merged

Add SocketEngineType.DefaultTransport#69
antonfirsov merged 2 commits into
tmds:masterfrom
antonfirsov:af/default-sockets

Conversation

@antonfirsov
Copy link
Copy Markdown
Collaborator

So we can use the original SocketsTransport as a baseline

Comment thread test/web/ConsoleLineArgumentsParser.cs Outdated
IOUringTransport,
LinuxTransport
LinuxTransport,
DefaultSockets
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe drop Default from the name?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure. In many cases such naming is used to indicate that a default option is being picked from the remaining enum members.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know what you mean by default, though it is not the default here.
Maybe: KestrelDefaultTransport?
Anyway, feel free to pick what you like and merge this.

Copy link
Copy Markdown
Collaborator Author

@antonfirsov antonfirsov Mar 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up with the name DefaultTransport, since other variants also lack the Kestrel* prefix.

Comment thread test/web/Program.cs
case SocketEngineType.DefaultSockets:
webBuilder.UseSockets(options =>
{
options.DispatchContinuations = commandLineOptions.DispatchContinuations.Value;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonfirsov fyi, we've added an option for DontAllocateMemoryForIdleConnections in dotnet/aspnetcore#19396.

Copy link
Copy Markdown
Owner

@tmds tmds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@antonfirsov antonfirsov changed the title Add SocketEngineType.DefaultSockets Add SocketEngineType.DefaultTransport Mar 21, 2020
@antonfirsov antonfirsov merged commit 5e8a008 into tmds:master Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants