Skip to content

Performance: Implement non-wildcard topic subscription management in MqttClientSessionsManager. #2175

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

AlexNik4
Copy link

@AlexNik4 AlexNik4 commented Apr 11, 2025

@AlexNik4 AlexNik4 marked this pull request as draft April 11, 2025 20:33
@AlexNik4
Copy link
Author

@dotnet-policy-service agree

@AlexNik4
Copy link
Author

AlexNik4 commented Apr 11, 2025

While this greatly improves performance for non-wildcard topics at the small cost of another index, there is still the issue of incorrectly excluding clients that care about this topic via wildcard subscriptions at the same time as other clients subscribe without wildcards. Tests pass though.

…c subscriptions. This fixes the scenario where a topic now correctly matches both a simple subscription and a wildcard subscription. This also improves performance where if a simple topic is not matched, then only the sessions with wildcard topics will be checked.
@AlexNik4
Copy link
Author

Ok. I think this version works for all cases. I am getting huge performance improvements when there are many clients subscribing to different non-wildcard topics. In the past 2k messages per second with 10k clients was failing. Now I can get 20k messages per second with 20k clients. This will not improve performance if all the clients are using wildcards. But it should not meaningfully degrade performance in that scenario either.

@AlexNik4
Copy link
Author

I can look into unit tests later. Would appreciate some feedback on this. Overall, this is just separating topics with and without wildcards so that we can perform additional optimizations for non wildcard topics.

@AlexNik4 AlexNik4 marked this pull request as ready for review April 12, 2025 03:19
@AlexNik4 AlexNik4 changed the title Performance: Implemented simple topic subscription management in MqttClientSessionsManager. Performance: Implemented non-wildcard topic subscription management in MqttClientSessionsManager. Apr 12, 2025
@AlexNik4 AlexNik4 changed the title Performance: Implemented non-wildcard topic subscription management in MqttClientSessionsManager. Performance: Implement non-wildcard topic subscription management in MqttClientSessionsManager. Apr 12, 2025
@chkr1011
Copy link
Collaborator

@logicaloud Please have a look at the change and let me know what you think.

@logicaloud
Copy link
Contributor

Looks feasible. @AlexNik4, maybe you could run benchmarks "MessageDeliveryBenchmark", "SubscribeBenchmark", and "UnsubscribeBenchmark" and compare the Before and After to cover other scenarios as well?

So far application message handling was geared towards the "many publishers, few subscribers" scenario (see #1298 and "Other considerations"). Having lower performance for a "many subscribers" scenario is not unexpected and improvements here would be great.

Some random observations:

  • The check for wildcards in OnSubscriptionAdded could be avoided by passing a list of MqttSubscription to the method instead of the list of strings. The MqttSubscription contains a TopicHasWildcard flag as well as the topic string. The Subscribe method would need to be modified to collect the MqttSubscription in createSubscriptionResult as addedSubscriptions.
  • Should the GetSimpleSubscribedTopics property be called SubscribedSimpleTopics?

@AlexNik4
Copy link
Author

AlexNik4 commented Apr 16, 2025

@logicaloud
Addressed your comments. I ran the benchmarks and am not seeing any real differences. Everything seems within margin of error:

SubscribeBenchmark

Original

Method Mean Error StdDev Gen0 Gen1 Allocated
Subscribe_10000_Topics 288.8 ms 8.68 ms 25.58 ms 3000.0000 1000.0000 51.33 MB

With Changes

Method Mean Error StdDev Gen0 Gen1 Allocated
Subscribe_10000_Topics 285.5 ms 7.29 ms 21.49 ms 3000.0000 1000.0000 51.34 MB

MessageDeliveryBenchmark

Original

Method NumPublishers NumSubscribedTopicsPerSubscriber NumSubscribers NumTopicsPerPublisher Mean Error StdDev Gen0 Gen1 Allocated
DeliverMessages 1000 5 10 1 480.4 us 9.59 us 20.43 us 13.6719 1.9531 228.66 KB
DeliverMessages 1000 5 10 5 478.3 us 9.47 us 20.97 us 13.6719 1.9531 228.76 KB
DeliverMessages 1000 10 10 1 938.5 us 18.55 us 41.12 us 27.3438 5.8594 459.67 KB
DeliverMessages 1000 10 10 5 925.1 us 18.38 us 45.09 us 27.3438 5.8594 451.22 KB
DeliverMessages 1000 20 10 1 1,849.5 us 36.97 us 92.76 us 54.6875 - 895.78 KB
DeliverMessages 1000 20 10 5 1,861.6 us 37.22 us 103.76 us 54.6875 11.7188 898.5 KB
DeliverMessages 1000 50 10 1 4,782.4 us 94.89 us 241.52 us 132.8125 - 2247.69 KB
DeliverMessages 1000 50 10 5 4,731.9 us 94.43 us 233.42 us 132.8125 46.8750 2233.23 KB
DeliverMessages 10000 5 10 1 603.8 us 12.05 us 27.94 us 13.6719 1.9531 234.84 KB
DeliverMessages 10000 5 10 5 593.9 us 11.82 us 26.18 us 13.6719 3.9063 234.4 KB
DeliverMessages 10000 10 10 1 1,164.1 us 22.59 us 47.15 us 27.3438 5.8594 459.29 KB
DeliverMessages 10000 10 10 5 1,165.0 us 23.19 us 45.23 us 27.3438 3.9063 461.97 KB
DeliverMessages 10000 20 10 1 2,348.2 us 46.93 us 107.83 us 54.6875 11.7188 924.58 KB
DeliverMessages 10000 20 10 5 2,353.1 us 46.76 us 116.46 us 58.5938 11.7188 968.89 KB
DeliverMessages 10000 50 10 1 6,105.9 us 120.60 us 180.50 us 148.4375 23.4375 2421.8 KB
DeliverMessages 10000 50 10 5 6,161.9 us 122.52 us 159.31 us 140.6250 23.4375 2310.38 KB

With Changes

Method NumPublishers NumSubscribedTopicsPerSubscriber NumSubscribers NumTopicsPerPublisher Mean Error StdDev Gen0 Gen1 Allocated
DeliverMessages 1000 5 10 1 482.5 us 9.60 us 20.67 us 12.6953 2.9297 213.55 KB
DeliverMessages 1000 5 10 5 475.0 us 9.39 us 21.21 us 12.6953 2.9297 211.04 KB
DeliverMessages 1000 10 10 1 941.9 us 18.69 us 45.50 us 25.3906 5.8594 419.38 KB
DeliverMessages 1000 10 10 5 930.3 us 18.46 us 43.50 us 25.3906 5.8594 418.25 KB
DeliverMessages 1000 20 10 1 1,864.9 us 37.06 us 93.64 us 50.7813 - 828.37 KB
DeliverMessages 1000 20 10 5 1,833.5 us 36.58 us 99.53 us 50.7813 7.8125 831.28 KB
DeliverMessages 1000 50 10 1 4,684.0 us 93.11 us 253.31 us 125.0000 39.0625 2068.47 KB
DeliverMessages 1000 50 10 5 4,769.9 us 94.45 us 240.41 us 125.0000 - 2047.32 KB
DeliverMessages 10000 5 10 1 595.0 us 11.33 us 26.04 us 12.6953 2.9297 216.32 KB
DeliverMessages 10000 5 10 5 595.7 us 11.74 us 19.30 us 13.6719 2.9297 226.41 KB
DeliverMessages 10000 10 10 1 1,159.4 us 23.13 us 45.66 us 25.3906 3.9063 427.29 KB
DeliverMessages 10000 10 10 5 1,158.9 us 22.79 us 49.06 us 25.3906 5.8594 427.56 KB
DeliverMessages 10000 20 10 1 2,313.5 us 46.01 us 96.03 us 50.7813 7.8125 841.08 KB
DeliverMessages 10000 20 10 5 2,297.8 us 45.38 us 101.50 us 50.7813 7.8125 845.53 KB
DeliverMessages 10000 50 10 1 5,971.3 us 114.60 us 200.72 us 125.0000 23.4375 2095.64 KB
DeliverMessages 10000 50 10 5 5,954.5 us 118.53 us 207.59 us 125.0000 31.2500 2099.18 KB

UnsubscribeBenchmark

Original

Method Mean Error StdDev Gen0 Allocated
Unsubscribe_10000_Topics 277.7 ms 7.66 ms 22.57 ms 3000.0000 48.82 MB

With Changes

Method Mean Error StdDev Median Gen0 Allocated
Unsubscribe_10000_Topics 275.8 ms 8.70 ms 25.64 ms 282.6 ms 3000.0000 48.81 MB

@logicaloud
Copy link
Contributor

Looks good to me. Up to @chkr1011 for review but here are some ideas regarding naming of variables that may convey the meaning a bit better.

Instead of _simpleTopicsToSession: _simpleTopicToSessions or _subscriberSessionsBySimpleTopic

Instead of out var sessionsWithSimpleTopics: out var simpleTopicSessions

@AlexNik4 AlexNik4 requested a review from chkr1011 April 30, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants