-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: master
Are you sure you want to change the base?
Conversation
…ClientSessionsManager.
@dotnet-policy-service agree |
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.
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. |
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. |
@logicaloud Please have a look at the change and let me know what you think. |
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:
|
@logicaloud SubscribeBenchmarkOriginal
With Changes
MessageDeliveryBenchmarkOriginal
With Changes
UnsubscribeBenchmarkOriginal
With Changes
|
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 Instead of |
#2174