Skip to content

threadshare: simplify some elements implementations

François Laignel requested to merge fengalin/gst-plugins-rs:ts-mutex-removal into main

While working on !778 (merged), I noticed some ts elements could be simplified by taking advantage of the TaskImpl being mutably accessible in the iteration function. This allows getting rid of some Mutexes in the async hot path. For some elements, this is achieved by moving fields and functions from the Pad{Src,Sink}Handlers to the TaskImpl. Other fields could also be moved to ElementImpl, which is easily accessible from the Pad{Src,Sink}Handlers and the TaskImpl. In most cases, the Pad{Src,Sink}Handlers is reduced to unit-like structs, which removes some boilerplates, Arcs & clones.

Most ts elements could be simplified using the above guidelines. However, ts-inputselector would require an improvement to the Task design. Currently, TaskImpl implementers are passed to the Task when the element is switched to the Ready state and they can't be retrieved afterwards. This prevents the TaskImpl impls from owning fields that could be instantiated at element construction and / or modified before the switch to Ready because we can't reuse the TaskImpl after a switch from Ready to Null, so we wouldn't be able to access these fields for a subsequent Null to Ready switch. I have ideas for a design to unlock this (and other Task improvements and simplifications), but that would be worth a dedicate MR in my opinion. I would only start working on this if the threadshare infrastructure is considered to have a future though! :)

I didn't touch ts-jitterbuffer yet, because I think it would benefit greatly from the improvements from the above § and also because there are pending changes in !756.

See first commits for details on the changes. One word about why I choosed flume as the channels for ts-udpsink though:

  • The send and receive functions can be called on immutable Senders and Receivers, so we don't need Mutexes for Senders (Receivers are mutably accessible).
  • The Receivers are clonable. We can build the Item & Command channels at element creation and clone the Receivers when the Task is prepared. Using regular futures channels wouldn't be a problem here if the Task design could allow reusing the TaskImpl after a switch from Ready to Null (see above).
  • flume channels are both usable in sync and async contexts. I also considered using crossbeam which is already a transitive dependency for gst-plugin-audiofx and which provides the same advantages for the 2 first points, but crossbeam can't asynchronously send nor receive.

Apart from code simplifications, I noticed CPU usage reductions ranging from marginal to significant depending on the test. This is very theoritical though as I don't have access to a real world test environment for this.

Fixes: #107 (closed) ... somewhat: I didn't bench all the lock primitives listed in this issue, but this MR removes Mutexes when possibles and avoids costly async Mutexes. Also RwLocks were replaced. Contention at an element level is very low so the balance is in favour of regular Mutexes.

Merge request reports