-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
use approved SocketAddress API instead of direct internal access #89841
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThis is cleanup to avoid using With all the external access to internal array gone, I'm wondering if it would be beneficial to have something like this @stephentoub to avoid creation of the memory on each access. private Memory<byte> _buffer = new Memory<byte>(new byte[Size]); contributes to #30797
|
I doubt it. That'd increase the size of the object, and the cost of constructing a Memory is minimal. Feel free to measure, though I expect it's entirely within the noise. |
src/libraries/Common/src/Interop/Unix/System.Native/Interop.Bind.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs
Outdated
Show resolved
Hide resolved
this is final stepping stone. Please take another look @stephentoub |
<Compile Include="$(CommonPath)System\Net\Sockets\ProtocolType.cs" | ||
Link="Common\System\Net\Sockets\ProtocolType.cs" /> | ||
<Compile Include="$(CommonPath)System\Net\Sockets\SocketType.cs" | ||
Link="Common\System\Net\Sockets\SocketType.cs" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do it later if you'd prefer, but these files should also move out of common into the appropriate library folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I was thinking about SocketAddress.cs
as well once the changes are over. Probably separate PR with just moves/renames.
This is cleanup to avoid using
SocketAddress
internals directly.Avoids allocation for local and remote address in typical case.
With all the external access to internal array gone, I'm wondering if it would be beneficial to have something like this @stephentoub to avoid creation of the memory on each access.
contributes to #30797
follow-up on #88970