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

DefaultHubLifetimeManager performs full enumeration of ConcurrentDictionary when calling SendUsersAsync (SendToAllConnections) #60092

Open
1 task done
Dimoner opened this issue Jan 29, 2025 · 2 comments
Labels
area-signalr Includes: SignalR clients and servers

Comments

@Dimoner
Copy link

Dimoner commented Jan 29, 2025

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Currently, when SendUsersAsync is called in DefaultHubLifetimeManager, it performs a full iteration over the ConcurrentDictionary, checking all connections instead of retrieving the data in O(1) time complexity.

This can lead to performance issues as the iteration scales poorly with the number of connections. Moreover, this behavior is not intuitive, since other hub lifetime managers, such as RedisHubLifetimeManager, create separate channels for each user instead of storing all users under a single key and performing a full scan.

Describe the solution you'd like

I propose adding a separate dictionary to store connections by userId, similar to how groups and connections are already managed:
private readonly HubConnectionStore _connections = new HubConnectionStore(); private readonly HubGroupList _groups = new HubGroupList();
This would allow direct access to user connections in O(1) time, significantly improving performance, especially in high-load scenarios.

Additional context

The current approach affects performance when scaling large SignalR applications.
RedisHubLifetimeManager already follows a per-user channel approach, making it more efficient.
Introducing a userId-based dictionary would align the behavior of DefaultHubLifetimeManager with RedisHubLifetimeManager, making the API more consistent.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-signalr Includes: SignalR clients and servers label Jan 29, 2025
@Dimoner
Copy link
Author

Dimoner commented Jan 29, 2025

If this approach sounds reasonable, I would be happy to implement the necessary changes and submit a PR. Let me know what you think!

@BrennanConroy
Copy link
Member

Just curious, are you actually seeing a performance issue with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

No branches or pull requests

2 participants