rebus-org / Rebus

:bus: Simple and lean service bus implementation for .NET
https://mookid.dk/category/rebus
Other
2.31k stars 359 forks source link

Performance Question #1145

Closed dazinator closed 6 months ago

dazinator commented 6 months ago

I notice that ThreadPoolWorker seems to sit in an infinite loop until cancelled.


 while (!token.IsCancellationRequested)
        {
            try
            {
                TryReceiveNextMessage(token);
            }
            catch (Exception exception)
            {
                _log.Error(exception, "Unhandled exception in worker {workerName} when try-receiving", Name);

                _backoffStrategy.WaitError(token);
            }
        }

Is this effectively keeping the thread occupied its entire time, not allowing the thread to pick up other work - a message pump basically?

Because having a dedicated thread seems a bit heavy handed where perhaps it would be ok for threads to be used from a thread pool when available? It appears like it would be constantly checking for new messages so you can dispatch them as speedily as possible - and I can see how that would be desirable.. however in many async messaging apps, perhaps its ok to have a small delay - for example, by using the default thread pool instead, and a Task.Delay between checks? WIth the caveat that, if you are low on threads, and have a lot of pending tasks, you are reliant on dotnet threadpool to scale threads and eventually the task will be picked up to check for messages.. i.e it seems like that would tradeoff "message dispatch immediacy" with potentially more efficient usage of CPU resources?

I haven't delved into any optimisations and this is not my area of expertise, so forgive me for any naivety, but in async scalable apps, I am used to having dotnet scale the number of threads in the thread pool, and trying not to consume threads for long periods, so when I see code that seems to consume a thread (even when they may not often be any work to do potentially) it rings alarm bells. Are my concerns valid?

dazinator commented 6 months ago

Is this perhaps something thats only relevent when running InMemory and gets replaced when for example using ServiceBus perhaps? (If so, then it's much less of a concern - I am seeing this class in local testing with an InMemory setup currently)

mookid8000 commented 6 months ago

This design came from back when Rebus was originally built, and back then it used MSMQ, which did not have an asynchronous receive API that was easy to work with.

Rebus simply starts a number of dedicated "worker threads" (default: 1), which it will then use to begin the receive operation. How many receive operations will then be determined by the "max parallelism" (default: 5), and if the transport supports truly async Task-based receive, then message dispatch will continue on the thread pool.

TBH I've never tried to dive into the performance characteristics of this design beyond from a usage perspective, where it just seems that Rebus does not consume much memory, does not consume much CPU when running idle, seems to have a low latency when receiving messages, and seems to be well-behaved towards other things running in the same process and/or on the same machine.

If this is something that interests you, please feel free to tear it apart, if you can 😉

Oh, and please note: If it wasn't entirely clear, the "worker threads" are DEDICATED worker threads, which Rebus creates, and so it doesn't affect .NET's thread pool or anything else. And the threads are created when Rebus starts, and they're stopped again when Rebus shuts down, so once they're there, they will stay there until Rebus stops.

dazinator commented 6 months ago

I found the docs saying that a TPL based approach is also provided and I think this makes a great deal of sense for transports that do support async receive, and it simplifies the concurrency setting by taking an option away :-)

https://github.com/rebus-org/Rebus/wiki/Workers-and-parallelism#tpl-based-workers