Socket.Unix: stackalloc IoVector/GCHandle for multi-buffer send/receives. by tmds · Pull Request #37583 · dotnet/runtime

tmds · 2020-06-08T08:30:32Z

cc @stephentoub @roji

…ves.

ghost · 2020-06-08T08:30:41Z

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

tmds · 2020-06-08T19:01:42Z

@EgorBo @stephentoub @bbartels thanks for reviewing!

stephentoub · 2020-06-08T20:30:35Z

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

-            int maxBuffers = buffers.Count;
-            var handles = new GCHandle[maxBuffers];
-            var iovecs = new Interop.Sys.IOVector[maxBuffers];
+            Span<GCHandle> handles = allocOnStack ? stackalloc GCHandle[IovStackThreshold] : new GCHandle[maxBuffers];


Since IovStackThreshold is so small, I wonder if it would be worth it to have a dedicated path for a small number of buffers and rather than using pinned GCHandles using fixed. The latter would necessitate writing out a fixed statement for each input, which is why it would need to be a small number, but it generally also has less overhead, in particular it's close to free when a GC doesn't occur, whereas there's always overhead to creating and free'ing GCHandles.

We can start with what you have and subsequently experiment with that if it's deemed a good idea, e.g. if we think two or three buffers for example is the most common case. Then again, this is all the synchronous code path, so maybe we needn't invest a ton in optimizing it further.

Sounds like an interesting idea to try. The relative gain will be small due to the high cost of the syscall. Are there some cases where this optimization is performed already in corefx?

Are there some cases where this optimization is performed already in corefx?

This one's pretty close:

runtime/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs

Lines 439 to 471 in 7a57b92

fixed (void* pinnedToken0 = inSecBuffers._item0.Token)

fixed (void* pinnedToken1 = inSecBuffers._item1.Token)

fixed (void* pinnedToken2 = inSecBuffers._item2.Token)

{

// Fix Descriptor pointer that points to unmanaged SecurityBuffers.

inSecurityBufferDescriptor.pBuffers = inUnmanagedBufferPtr;

// Updated pvBuffer with pinned address. UnmanagedToken takes precedence.

if (inSecBuffers.Count > 2)

{

inUnmanagedBuffer[2].BufferType = inSecBuffers._item2.Type;

inUnmanagedBuffer[2].cbBuffer = inSecBuffers._item2.Token.Length;

inUnmanagedBuffer[2].pvBuffer = inSecBuffers._item2.UnmanagedToken != null ?

(IntPtr)inSecBuffers._item2.UnmanagedToken.DangerousGetHandle() :

(IntPtr)pinnedToken2;

}

if (inSecBuffers.Count > 1)

{

inUnmanagedBuffer[1].BufferType = inSecBuffers._item1.Type;

inUnmanagedBuffer[1].cbBuffer = inSecBuffers._item1.Token.Length;

inUnmanagedBuffer[1].pvBuffer = inSecBuffers._item1.UnmanagedToken != null ?

(IntPtr)inSecBuffers._item1.UnmanagedToken.DangerousGetHandle() :

(IntPtr)pinnedToken1;

}

if (inSecBuffers.Count > 0)

{

inUnmanagedBuffer[0].BufferType = inSecBuffers._item0.Type;

inUnmanagedBuffer[0].cbBuffer = inSecBuffers._item0.Token.Length;

inUnmanagedBuffer[0].pvBuffer = inSecBuffers._item0.UnmanagedToken != null ?

(IntPtr)inSecBuffers._item0.UnmanagedToken.DangerousGetHandle() :

(IntPtr)pinnedToken0;

}

I could have sworn we did something like this for WSABuffers on Windows, but I can't find it now. Likely it was an experiment that never got checked in.

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

tmds · 2020-06-09T10:25:05Z

CI shows System.Net.Sockets.Tests Work Item failed, so probably something is wrong with this PR. I'll investigate.

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

…#37583") (#40753) * Revert " Use clonefile for CopyFile, if available (dotnet/corefx#37583)" This reverts commit 02ba75a. * Add unit test * Disable tests failing in browser-wasm

Socket.Unix: stackalloc IoVector/GCHandle for multi-buffer send/recei…

7994523

…ves.

Dotnet-GitSync-Bot added the area-System.Net.Sockets label Jun 8, 2020

tmds commented Jun 8, 2020

View reviewed changes