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

SignalR performance: track groups per connection, remove on disconnect #53486

Merged
merged 10 commits into from
Feb 7, 2024
30 changes: 29 additions & 1 deletion src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ public override Task AddToGroupAsync(string connectionId, string groupName, Canc
return Task.CompletedTask;
}

// Track groups in the connection object
lock (connection.GroupNames)
{
if (!connection.GroupNames.Add(groupName))
{
// Connection already in group
return Task.CompletedTask;
}
}

_groups.Add(connection, groupName);
// Connection disconnected while adding to group, remove it in case the Add was called after OnDisconnectedAsync removed items from the group
if (connection.ConnectionAborted.IsCancellationRequested)
Expand All @@ -64,6 +74,16 @@ public override Task RemoveFromGroupAsync(string connectionId, string groupName,
return Task.CompletedTask;
}

// Remove from previously saved groups
lock (connection.GroupNames)
{
if (!connection.GroupNames.Remove(groupName))
{
// Connection not in group
return Task.CompletedTask;
}
}

alex-jitbit marked this conversation as resolved.
Show resolved Hide resolved
_groups.Remove(connectionId, groupName);

return Task.CompletedTask;
Expand Down Expand Up @@ -277,8 +297,16 @@ public override Task OnConnectedAsync(HubConnectionContext connection)
/// <inheritdoc />
public override Task OnDisconnectedAsync(HubConnectionContext connection)
{
lock (connection.GroupNames)
{
// Remove from tracked groups one by one
foreach (var groupName in connection.GroupNames)
{
_groups.Remove(connection.ConnectionId, groupName);
}
}

_connections.Remove(connection);
_groups.RemoveDisconnectedConnection(connection.ConnectionId);

return Task.CompletedTask;
}
Expand Down
4 changes: 4 additions & 0 deletions src/SignalR/server/Core/src/HubConnectionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics.CodeAnalysis;
using System.IO.Pipelines;
using System.Security.Claims;
using System.Text.RegularExpressions;
alex-jitbit marked this conversation as resolved.
Show resolved Hide resolved
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Connections.Abstractions;
using Microsoft.AspNetCore.Connections.Features;
Expand Down Expand Up @@ -56,6 +57,9 @@ public partial class HubConnectionContext
[MemberNotNullWhen(true, nameof(_messageBuffer))]
internal bool UsingStatefulReconnect() => _useStatefulReconnect;

// Tracks groups that the connection has been added to
internal HashSet<string> GroupNames { get; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
alex-jitbit marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Initializes a new instance of the <see cref="HubConnectionContext"/> class.
/// </summary>
Expand Down
10 changes: 0 additions & 10 deletions src/SignalR/server/Core/src/Internal/HubGroupList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections;
using System.Collections.Concurrent;
using System.Linq;

namespace Microsoft.AspNetCore.SignalR.Internal;

Expand Down Expand Up @@ -43,15 +42,6 @@ public void Remove(string connectionId, string groupName)
}
}

public void RemoveDisconnectedConnection(string connectionId)
{
var groupNames = _groups.Where(x => x.Value.ContainsKey(connectionId)).Select(x => x.Key);
foreach (var groupName in groupNames)
{
Remove(connectionId, groupName);
}
}

public int Count => _groups.Count;

public IEnumerator<ConcurrentDictionary<string, HubConnectionContext>> GetEnumerator()
Expand Down