GNOME Bugzilla – Bug 621299
make simple queues faster
Last modified: 2010-08-30 06:50:45 UTC
Instead of making it a separate element, we can probably add a property to queue (silent) to skip : unlock(); signal_emit(); lock(); in silent mode.
On the other hand I was thinking about adding a gboolean passthrough property. This would change threadbarrier into identity. Dynamically (de)activating the loop-function needs some thoughts though. The use case is e.g. playbin2 where we currently plug a queue between videodecoder and sink. Some decoders (e.g. dsp decoders on N900) alreday have a queue-like behaviour and the extra queue increases the pressure on the pad_alloced buffers (needs more).
Another usecase is demuxer/tee elements where one just wants the thread-boundary but no buffering per se.
Created attachment 163389 [details] [review]
add new threadboundary element
hmm... why not just use queue max-size-buffers=1 ?
(In reply to comment #2)
> hmm... why not just use queue max-size-buffers=1 ?
Instead of what?
Instead of the new element or queue property
Just using max-size-buffers=1 would not be enough. We would need to overload the semantics and supress the signalling in that case too.
The savings come from the less locking on the element side, but also to the less locking from the gobject side.
queue doesn't use gobject signaling internally anymore. The size calculation routines could also be shunted if in the max-size-buffers==1 case.
I'd much prefer optimizing the hell out of queue in that case than having another element which is so similar to queue.
> Another usecase is demuxer/tee elements where one just wants the
> thread-boundary but no buffering per se.
Not sure this is correct to assume in general. It may be true for some very specific use cases, but very often you will still want queues which can hold more than 1 buffer after demuxers and tee.
I'd rather improve queue/multiqueue etc. than introduce yet another element that can only be used in very specific circumstances (as if this stuff wasn't hard enough already).
Regarding the playbin2 case, where there is a very small queue in front of the sinks, we could just add another flag to make playbin2 not plug these, for sinks that have internal queues.
(In reply to comment #6)
> queue doesn't use gobject signaling internally anymore.
Sure it does. It provides 4 signals - underrun, running, overrun and pushing.
> The size calculation
> routines could also be shunted if in the max-size-buffers==1 case.
> I'd much prefer optimizing the hell out of queue in that case than having
> another element which is so similar to queue.
That would be okay for me as well. I was mainly doing the elements also to see what potential gains we can get still.
(In reply to comment #7)
> Not sure this is correct to assume in general. It may be true for some very
> specific use cases, but very often you will still want queues which can hold
> more than 1 buffer after demuxers and tee.
The element in the patch is not limmited to 1 buffer.
> I'd rather improve queue/multiqueue etc. than introduce yet another element
> that can only be used in very specific circumstances (as if this stuff wasn't
> hard enough already).
I actualy belive tee should just start new tasks for each pad and use queues internaly (we could have a queue helper like adapter). If we would have a demuxer baseclas then this could have the multi-queue functuality. But this is maybe something for 0.11.
> Regarding the playbin2 case, where there is a very small queue in front of the
> sinks, we could just add another flag to make playbin2 not plug these, for
> sinks that have internal queues.
The flag in playbin2 would be not so nice, as it pushes the decision to the application (of course this is not anyhow better if we have a queue that can be deactivated). Now I am thinking that ideally we have some query (?) / flag on elements to be able to figure whether the have a linear output behaviour or not.
I'll try to see if I can easily achieve the same performance improvement in regular queue (e.g. by adding a "no-signals" / "silent" property).
Created attachment 168876 [details] [review]
add silent property to suppress signal emission
Different approach as promissed.
$ sh ./testqueue.sh
46.47user 16.20system 0:43.20elapsed 145%CPU
46.17user 15.92system 0:42.76elapsed 145%CPU
46.26user 15.85system 0:42.86elapsed 144%CPU
don't emit signals
44.55user 15.08system 0:40.95elapsed 145%CPU
44.14user 16.00system 0:41.59elapsed 144%CPU
44.36user 16.12system 0:41.85elapsed 144%CPU
~ 5 % speedup
Arm Cortex A8
real 5m 17.27s user 4m 7.53s sys 0m 59.38s
real 5m 9.89s user 4m 7.42s sys 0m 58.89s
real 5m 12.30s user 4m 5.65s sys 1m 1.55s
don't emit signals
real 5m 0.73s user 4m 0.74s sys 0m 55.91s
real 4m 55.88s user 3m 56.30s sys 0m 56.10s
real 4m 58.83s user 3m 54.02s sys 1m 0.25s
~ 10 % speedup
Author: Stefan Kost <firstname.lastname@example.org>
Date: Fri Aug 27 15:35:49 2010 +0300
queue: add silent property to suppress signal emission
Allow to turn off signal emission and therefore extra locking if this is not needed.