KallDrexx / rust-media-libs

Rust based libraries for misc media functionality
Apache License 2.0
229 stars 58 forks source link

threaded rtmp server example will spin wait resulting in high CPU load #44

Open Christiaan676 opened 1 year ago

Christiaan676 commented 1 year ago

In the threaded_rtmp_server example, the function handle_connections will poll the channels as fast as it can resulting in a 100% CPU load for that thread.

The function handle_connections calls connection_receiver.try_recv() and connection.read() (containing reader.try_recv()) in a loop. As there is no blocking operation, the loop will spin wait until a new connection is established or data is received.

On solution would be to use the crossbeam select macro on the two channels. Though that would require exposing the channel in the connection.read() function.

Christiaan676 commented 1 year ago

It's not possible to do a select on a dynamic list of channels. So that won't solve the issue. So not sure on how to solve this .....

KallDrexx commented 1 year ago

Out of curiosity, is there a reason you are trying to work with the threaded example instead of the tokio example? Thread per connection is probably not the most efficient way to handle it.

In any case, looking at the threaded rtmp server code, I'm not sure what I was doing there. It almost looks like I tried to follow the mio example exactly but without mio. In other words, it wants to keep everything on a single thread if it can instead of utilizing multiple threads.

The only thing I can think of is I wrote this before I really discovered actor patterns. Instead of one monolithic handle_connections() thread, we need

When a connection comes in, the accepting thread creates the connection management thread. The connection management thread starts the RTMP server session, then creates the reading and writing threads. The reading thread sends raw bytes to the connection management thread via a channel as it receives them, and anytime the connection management thread has bytes to send it sends it to the TCP writing thread via another channel.

The 1 thread to track who is publishing/subscribing has it's own channel that receives and relays data from RTMP . When a publisher connects to a stream key, the management thread will send a message to the pub/sub management thread via its channel. If the publisher sends some Video data, the connection management thread sends the VideoData{} over the channel to the pubsub thread, which the nrelays it to any connection management thread channels for clients connected to the same stream key.

Essentially, each thread is an actor which self manages it's own state and communicates with the other systems via channels. This also allows each of these threads to be testable in isolation without network I/O.

Hopefully that makes sense

It's not possible to do a select on a dynamic list of channels. So that won't solve the issue. So not sure on how to solve this .....

I'm not sure how you handle this well in threaded/non-async land. In non-async land you have the bad approach of using a FuturesUnorderd<T>, but that has a lot of problems.

The one way I handle this in Tokio is using multiple channels. So if I have N channels I need my thread to read from, I put the recv() for each channel in it's own tokio task, and when that channel receives data I then send it to a centralized channel that the thread receives from. This means the centralized thread/task can read from just one channel, even though it's spawning tokio tasks to read from a dynamic number of channels.

This is easy with tokio, and I have performance benchmarks showing that this is well performant and low allocation. I'm not sure how well this performs with threads instead of async tasks though.

Christiaan676 commented 1 year ago

Agree that the threaded example is probably not the most efficient, and an aSync implementation would be better suited. Unfortunately I have to integrate a RTMP server into a threaded framework. This means that I need to create an aSync sync bridge. Or use the threaded approach, considering the use case is not requiring a lot of connections they sync approach seams fine. I hope you don't mind the merge requests, with small updates to improve the code.

Indeed the best approach is to create one channel with multiple producers. The connections can than send video data containing there channel ID over the channel. And the handle_connections function can select om the two channels.

Don't think you need 3 threads per connection. As far as I can tell the receiver thread can do the connection management, and the initialization.