Skip to content

Commit 00506eb

Browse files
authored
Remove ConcurrentDictionary lookups from Unix socket event loop (#109052)
* Remove ConcurrentDictionary lookups from Unix socket event loop * Switch from GCHandle to simple array+index
1 parent 9e6b45c commit 00506eb

File tree

4 files changed

+59
-38
lines changed

4 files changed

+59
-38
lines changed

src/libraries/System.Net.Sockets/src/Resources/Strings.resx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,6 @@
312312
<data name="SystemNetSockets_PlatformNotSupported" xml:space="preserve">
313313
<value>System.Net.Sockets is not supported on this platform.</value>
314314
</data>
315-
<data name="net_sockets_handle_already_used" xml:space="preserve">
316-
<value>Handle is already used by another Socket.</value>
317-
</data>
318315
<data name="net_sockets_address_small" xml:space="preserve">
319316
<value>Provided SocketAddress is too small for given AddressFamily.</value>
320317
</data>

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,8 @@ public void Trace(SocketAsyncContext context, string message, [CallerMemberName]
12621262
private SocketAsyncEngine? _asyncEngine;
12631263
private bool IsRegistered => _asyncEngine != null;
12641264
private bool _isHandleNonBlocking = OperatingSystem.IsWasi(); // WASI sockets are always non-blocking, because we don't have another thread which could be blocked
1265+
/// <summary>An index into <see cref="SocketAsyncEngine"/>'s table of all contexts that are currently <see cref="IsRegistered"/>.</summary>
1266+
internal int GlobalContextIndex = -1;
12651267

12661268
private readonly object _registerLock = new object();
12671269

@@ -1330,7 +1332,10 @@ public bool StopAndAbort()
13301332
// We don't need to synchronize with Register.
13311333
// This method is called when the handle gets released.
13321334
// The Register method will throw ODE when it tries to use the handle at this point.
1333-
_asyncEngine?.UnregisterSocket(_socket.DangerousGetHandle(), this);
1335+
if (IsRegistered)
1336+
{
1337+
SocketAsyncEngine.UnregisterSocket(this);
1338+
}
13341339

13351340
return aborted;
13361341
}

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

Lines changed: 52 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Concurrent;
5+
using System.Collections.Generic;
56
using System.Diagnostics;
67
using System.Runtime.CompilerServices;
78
using System.Runtime.InteropServices;
@@ -74,14 +75,17 @@ private static SocketAsyncEngine[] CreateEngines()
7475
return engines;
7576
}
7677

78+
/// <summary>
79+
/// Each <see cref="SocketAsyncContext"/> is assigned an index into this table while registered with a <see cref="SocketAsyncEngine"/>.
80+
/// <para>The index is used as the <see cref="Interop.Sys.SocketEvent.Data"/> to quickly map events to <see cref="SocketAsyncContext"/>s.</para>
81+
/// <para>It is also stored in <see cref="SocketAsyncContext.GlobalContextIndex"/> so that we can efficiently remove it when unregistering the socket.</para>
82+
/// </summary>
83+
private static SocketAsyncContext?[] s_registeredContexts = [];
84+
private static readonly Queue<int> s_registeredContextsFreeList = [];
85+
7786
private readonly IntPtr _port;
7887
private readonly Interop.Sys.SocketEvent* _buffer;
7988

80-
//
81-
// Maps handle values to SocketAsyncContext instances.
82-
//
83-
private readonly ConcurrentDictionary<IntPtr, SocketAsyncContextWrapper> _handleToContextMap = new ConcurrentDictionary<IntPtr, SocketAsyncContextWrapper>();
84-
8589
//
8690
// Queue of events generated by EventLoop() that would be processed by the thread pool
8791
//
@@ -119,28 +123,54 @@ public static bool TryRegisterSocket(IntPtr socketHandle, SocketAsyncContext con
119123

120124
private bool TryRegisterCore(IntPtr socketHandle, SocketAsyncContext context, out Interop.Error error)
121125
{
122-
bool added = _handleToContextMap.TryAdd(socketHandle, new SocketAsyncContextWrapper(context));
123-
if (!added)
126+
Debug.Assert(context.GlobalContextIndex == -1);
127+
128+
lock (s_registeredContextsFreeList)
124129
{
125-
// Using public SafeSocketHandle(IntPtr) a user can add the same handle
126-
// from a different Socket instance.
127-
throw new InvalidOperationException(SR.net_sockets_handle_already_used);
130+
if (!s_registeredContextsFreeList.TryDequeue(out int index))
131+
{
132+
int previousLength = s_registeredContexts.Length;
133+
int newLength = Math.Max(4, 2 * previousLength);
134+
135+
Array.Resize(ref s_registeredContexts, newLength);
136+
137+
for (int i = previousLength + 1; i < newLength; i++)
138+
{
139+
s_registeredContextsFreeList.Enqueue(i);
140+
}
141+
142+
index = previousLength;
143+
}
144+
145+
Debug.Assert(s_registeredContexts[index] is null);
146+
147+
s_registeredContexts[index] = context;
148+
context.GlobalContextIndex = index;
128149
}
129150

130151
error = Interop.Sys.TryChangeSocketEventRegistration(_port, socketHandle, Interop.Sys.SocketEvents.None,
131-
Interop.Sys.SocketEvents.Read | Interop.Sys.SocketEvents.Write, socketHandle);
152+
Interop.Sys.SocketEvents.Read | Interop.Sys.SocketEvents.Write, context.GlobalContextIndex);
132153
if (error == Interop.Error.SUCCESS)
133154
{
134155
return true;
135156
}
136157

137-
_handleToContextMap.TryRemove(socketHandle, out _);
158+
UnregisterSocket(context);
138159
return false;
139160
}
140161

141-
public void UnregisterSocket(IntPtr socketHandle, SocketAsyncContext __)
162+
public static void UnregisterSocket(SocketAsyncContext context)
142163
{
143-
_handleToContextMap.TryRemove(socketHandle, out _);
164+
Debug.Assert(context.GlobalContextIndex >= 0);
165+
Debug.Assert(ReferenceEquals(s_registeredContexts[context.GlobalContextIndex], context));
166+
167+
lock (s_registeredContextsFreeList)
168+
{
169+
s_registeredContexts[context.GlobalContextIndex] = null;
170+
s_registeredContextsFreeList.Enqueue(context.GlobalContextIndex);
171+
}
172+
173+
context.GlobalContextIndex = -1;
144174
}
145175

146176
private SocketAsyncEngine()
@@ -324,13 +354,11 @@ private readonly struct SocketEventHandler
324354
{
325355
public Interop.Sys.SocketEvent* Buffer { get; }
326356

327-
private readonly ConcurrentDictionary<IntPtr, SocketAsyncContextWrapper> _handleToContextMap;
328357
private readonly ConcurrentQueue<SocketIOEvent> _eventQueue;
329358

330359
public SocketEventHandler(SocketAsyncEngine engine)
331360
{
332361
Buffer = engine._buffer;
333-
_handleToContextMap = engine._handleToContextMap;
334362
_eventQueue = engine._eventQueue;
335363
}
336364

@@ -340,10 +368,15 @@ public bool HandleSocketEvents(int numEvents)
340368
bool enqueuedEvent = false;
341369
foreach (var socketEvent in new ReadOnlySpan<Interop.Sys.SocketEvent>(Buffer, numEvents))
342370
{
343-
if (_handleToContextMap.TryGetValue(socketEvent.Data, out SocketAsyncContextWrapper contextWrapper))
344-
{
345-
SocketAsyncContext context = contextWrapper.Context;
371+
Debug.Assert((uint)socketEvent.Data < (uint)s_registeredContexts.Length);
346372

373+
// The context may be null if the socket was unregistered right before the event was processed.
374+
// The slot in s_registeredContexts may have been reused by a different context, in which case the
375+
// incorrect socket will notice that no information is available yet and harmlessly retry, waiting for new events.
376+
SocketAsyncContext? context = s_registeredContexts[(uint)socketEvent.Data];
377+
378+
if (context is not null)
379+
{
347380
if (context.PreferInlineCompletions)
348381
{
349382
context.HandleEventsInline(socketEvent.Events);
@@ -365,18 +398,6 @@ public bool HandleSocketEvents(int numEvents)
365398
}
366399
}
367400

368-
// struct wrapper is used in order to improve the performance of the epoll thread hot path by up to 3% of some TechEmpower benchmarks
369-
// the goal is to have a dedicated generic instantiation and using:
370-
// System.Collections.Concurrent.ConcurrentDictionary`2[System.IntPtr,System.Net.Sockets.SocketAsyncContextWrapper]::TryGetValueInternal(!0,int32,!1&)
371-
// instead of:
372-
// System.Collections.Concurrent.ConcurrentDictionary`2[System.IntPtr,System.__Canon]::TryGetValueInternal(!0,int32,!1&)
373-
private readonly struct SocketAsyncContextWrapper
374-
{
375-
public SocketAsyncContextWrapper(SocketAsyncContext context) => Context = context;
376-
377-
internal SocketAsyncContext Context { get; }
378-
}
379-
380401
private readonly struct SocketIOEvent
381402
{
382403
public SocketAsyncContext Context { get; }

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Wasi.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ public static bool TryRegisterSocket(IntPtr socketHandle, SocketAsyncContext con
3535
return true;
3636
}
3737

38-
#pragma warning disable CA1822
39-
public void UnregisterSocket(IntPtr _, SocketAsyncContext context)
40-
#pragma warning restore CA1822
38+
public static void UnregisterSocket(SocketAsyncContext context)
4139
{
4240
context.unregisterPollHook.Cancel();
4341
}

0 commit comments

Comments
 (0)