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

Issues #2091 Improve the performance of RetainedMessages without subs… #2093

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zhaowgit
Copy link

Issues #2091 Improve the performance of RetainedMessages without subscribing to RetainedMessageChangedEvent.

…t subscribing to RetainedMessageChangedEvent.
@chkr1011
Copy link
Collaborator

Could you please add a Unit Tests which covers your situation? I am curious if we can make it fast even with an attached event handler (which is often the case) by converting the List to a ReadOnlyCollection etc. I am also wondering if the copy of the items to the list causes the issue or the fact that the async lock is acquired before (trying) to invoke the event.

@zhaowgit
Copy link
Author

zhaowgit commented Oct 28, 2024

using MQTTnet;
using MQTTnet.Server;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;
using System.Threading.Tasks;

namespace DotNet5WpfTest
{
    public class MQTTnet_Test
    {
        public static async Task RetainedMessages_Test(bool retain, bool subscribe, int amount)
        {
            var serverFactory = new MqttServerFactory();
            var options = serverFactory.CreateServerOptionsBuilder()
                .WithDefaultEndpoint()
                .WithDefaultEndpointPort(12346)
                .Build();
            var server = serverFactory.CreateMqttServer(options);
            await server.StartAsync();

            if (subscribe)
            {
                server.RetainedMessageChangedAsync += Server_RetainedMessageChangedAsync;
            }

            var testData = new Dictionary<string, string>();

            for (int i = 0; i < 3; i++)
            {
                for (int j = 0; j < amount; j++)
                {
                    var topic = $"RetainTest/{j:D6}";
                    testData[topic] = ($"{{\"topic\": \"{topic}\", \"value\": \"{j + i}\"}}");
                }

                var sw = Stopwatch.StartNew();
                foreach (var data in testData)
                {
                    await server.InjectApplicationMessage(new(new MqttApplicationMessageBuilder().WithTopic(data.Key).WithPayload(Encoding.UTF8.GetBytes(data.Value)).WithRetainFlag(retain).Build()));
                }
                sw.Stop();
                Debug.WriteLine($"Count:{i} Retain: {retain}, Subscribe:{subscribe}, Amount: {amount}, Time: {sw.ElapsedMilliseconds} ms");

                await Task.Delay(1000);
            }

            await server.StopAsync();
        }

        private static async Task Server_RetainedMessageChangedAsync(RetainedMessageChangedEventArgs arg)
        {
            return;
        }
    }
}

The output before this commit.
Count:0 Retain: True, Subscribe:True, Amount: 100000, Time: 40254 ms
Count:1 Retain: True, Subscribe:True, Amount: 100000, Time: 105772 ms
Count:2 Retain: True, Subscribe:True, Amount: 100000, Time: 106438 ms

Count:0 Retain: False, Subscribe:True, Amount: 100000, Time: 36 ms
Count:1 Retain: False, Subscribe:True, Amount: 100000, Time: 66 ms
Count:2 Retain: False, Subscribe:True, Amount: 100000, Time: 50 ms

Count:0 Retain: True, Subscribe:False, Amount: 100000, Time: 41163 ms
Count:1 Retain: True, Subscribe:False, Amount: 100000, Time: 103599 ms
Count:2 Retain: True, Subscribe:False, Amount: 100000, Time: 107543 ms

Count:0 Retain: False, Subscribe:False, Amount: 100000, Time: 49 ms
Count:1 Retain: False, Subscribe:False, Amount: 100000, Time: 51 ms
Count:2 Retain: False, Subscribe:False, Amount: 100000, Time: 56 ms

The output after this commit.
Count:0 Retain: True, Subscribe:True, Amount: 100000, Time: 39066 ms
Count:1 Retain: True, Subscribe:True, Amount: 100000, Time: 100003 ms
Count:2 Retain: True, Subscribe:True, Amount: 100000, Time: 99984 ms

Count:0 Retain: False, Subscribe:True, Amount: 100000, Time: 45 ms
Count:1 Retain: False, Subscribe:True, Amount: 100000, Time: 52 ms
Count:2 Retain: False, Subscribe:True, Amount: 100000, Time: 53 ms

Count:0 Retain: True, Subscribe:False, Amount: 100000, Time: 122 ms
Count:1 Retain: True, Subscribe:False, Amount: 100000, Time: 79 ms
Count:2 Retain: True, Subscribe:False, Amount: 100000, Time: 96 ms

Count:0 Retain: False, Subscribe:False, Amount: 100000, Time: 49 ms
Count:1 Retain: False, Subscribe:False, Amount: 100000, Time: 54 ms
Count:2 Retain: False, Subscribe:False, Amount: 100000, Time: 63 ms

ValueCollection is not IList, cannot use the constructor of ReadOnlyCollection.
I think we can optimize the performance when Retain: True, Subscribe:False first.

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.

2 participants