After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 621299 - make simple queues faster
make simple queues faster
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other Linux
: Normal enhancement
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-06-11 13:55 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2010-08-30 06:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add new threadboundary element (39.19 KB, patch)
2010-06-11 13:56 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
add silent property to suppress signal emission (5.72 KB, patch)
2010-08-27 13:19 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-11 13:55:27 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.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-11 13:56:02 UTC
Created attachment 163389 [details] [review]
add new threadboundary element
Comment 2 Edward Hervey 2010-06-11 14:09:44 UTC
hmm... why not just use queue max-size-buffers=1 ?
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-11 14:19:54 UTC
(In reply to comment #2)
> hmm... why not just use queue max-size-buffers=1 ?
Instead of what?
Comment 4 Sebastian Dröge (slomo) 2010-06-11 14:34:57 UTC
Instead of the new element or queue property
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-11 15:12:35 UTC
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.
Comment 6 Edward Hervey 2010-06-11 18:32:36 UTC
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.
Comment 7 Tim-Philipp Müller 2010-06-11 18:39:32 UTC
> 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.
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2010-06-12 20:23:55 UTC
(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).
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2010-08-27 13:19:47 UTC
Created attachment 168876 [details] [review]
add silent property to suppress signal emission

Different approach as promissed. 

X86

$ sh ./testqueue.sh 
emit signals
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

sh ./testqueue.sh
emit signals
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
Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2010-08-30 06:49:45 UTC
commit e406f7c57247660fe288a6577b3034bb3f692edd
Author: Stefan Kost <ensonic@users.sf.net>
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.
    Fixes #621299