GNOME Bugzilla – Bug 746462
rtpsession/rtpjitterbuffer: Use a thread pool to share all the timeout threads between elements
Last modified: 2018-05-04 09:39:16 UTC
See patch. This allows us to keep the number of threads much lower if there are multiple RTP sessions (or other users of GThreadPool).
Created attachment 299830 [details] [review] rtpsession: Use a thread pool to share all the RTCP sending threads between elements Usually the RTCP sending thread is idle, and during that times could easily be used by other rtpsessions for sending or even other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpsessions much lower, while still making it possible to have up to 1 thread per session if really needed.
I'm doing the same with jitterbuffer currently btw, will attach that patch here too.
Created attachment 299852 [details] [review] rtpjitterbuffer: Use a thread pool to share all timeout threads between elements Unless there's packet loss or other events that need our attention, the timeout thread is usually idle and could be used by other rtpjitterbuffers or any other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpjitterbuffers much lower, while still making it possible to have up to 1 thread per jitterbuffer if really needed.
The patches are breaking the unit tests, but otherwise seem to mostly work. Will investigate.
Can you also check with the Farstream unit tests (just do "make check"), they exercise the rtp stuff quite a bit.
Created attachment 299858 [details] [review] rtpsession: Use a thread pool to share all the RTCP sending threads between elements Usually the RTCP sending thread is idle, and during that times could easily be used by other rtpsessions for sending or even other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpsessions much lower, while still making it possible to have up to 1 thread per session if really needed.
Created attachment 299859 [details] [review] rtpjitterbuffer: Use a thread pool to share all timeout threads between elements Unless there's packet loss or other events that need our attention, the timeout thread is usually idle and could be used by other rtpjitterbuffers or any other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpjitterbuffers much lower, while still making it possible to have up to 1 thread per jitterbuffer if really needed.
(In reply to Olivier Crête from comment #5) > Can you also check with the Farstream unit tests (just do "make check"), > they exercise the rtp stuff quite a bit. The farstream testsuite did not pass all for me, independent of my patches :) The remaining changes needed for my patches for now are to make the unit tests a bit less dependent on the internal behaviour of the elements. The unit test assumes that the element is requesting clock ids in a specific way
Created attachment 303641 [details] [review] rtpsession: Use a thread pool to share all the RTCP sending threads between elements Usually the RTCP sending thread is idle, and during that times could easily be used by other rtpsessions for sending or even other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpsessions much lower, while still making it possible to have up to 1 thread per session if really needed.
Created attachment 303642 [details] [review] rtpjitterbuffer: Use a thread pool to share all timeout threads between elements Unless there's packet loss or other events that need our attention, the timeout thread is usually idle and could be used by other rtpjitterbuffers or any other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpjitterbuffers much lower, while still making it possible to have up to 1 thread per jitterbuffer if really needed.
Created attachment 303650 [details] [review] rtpjitterbuffer: Use a thread pool to share all timeout threads between elements Unless there's packet loss or other events that need our attention, the timeout thread is usually idle and could be used by other rtpjitterbuffers or any other users of non-exclusive GThreadPools. This allows us to keep the number of threads in scenarios with many rtpjitterbuffers much lower, while still making it possible to have up to 1 thread per jitterbuffer if really needed. Also fix the unit test. We now directly handle timeouts that are too late anyway instead of doing a roundtrip via the clock, so the test can't see any related clock ids anymore.
This all does not seem very useful with latest GLib anymore. Threads are only migrated from a thread pool to the global one after they were idle for >=15 seconds. So no real sharing happens here unless there is nothing to do here for >=15 seconds, which is rather unlikely. I think this was different some GLib versions ago, where threads immediately went back to the global thread pool and thus could be shared. What we could do here instead now would be to have one thread pool for all instances, but there we could easily cause too many threads (> number of instances) to be started.
This approach has proven to be useless, but Mathieu will provide a new approach for both in the next days :)