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 500653 - reuse qos event in basesink
reuse qos event in basesink
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: git master
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-11-30 14:54 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2009-05-07 13:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
reuse qos event (2.35 KB, patch)
2007-11-30 14:58 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
reuse qos event (2.19 KB, patch)
2007-12-03 07:19 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
rejected Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2007-11-30 14:54:22 UTC
Basesink creates a new qos event for each frame. It would be nice to reduce the number of allocations during playtime. The attached patche does that.

I meassured with this pipeline:
MP_LOG_START=0.3 G_SLICE=always-malloc G_DEBUG=gc-friendly LD_PRELOAD=libmempattern.so gst-launch-0.10 --gst-disable-registry-fork videotestsrc num-buffers=50 ! xvimagesink

and got this result:
$ # before
$ wc -l /tmp/mp_use.log
35406 /tmp/mp_use.log
$ # after
$ wc -l /tmp/mp_use.log 
34777 /tmp/mp_use.log
$ # savings = 629
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2007-11-30 14:58:43 UTC
Created attachment 99905 [details] [review]
reuse qos event
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2007-11-30 15:16:30 UTC
Okay, this is not the right way to solve the issue, as there can be multiple qos events flowing up the pipeline.

A better solution would be to have object caches. This applies to GstMiniObject subclasses (GstBuffer, GstMessage, GstEvent). The recycling could be done by using a free-list with initial allocations:
http://www.pulseaudio.org/browser/trunk/src/pulsecore/flist.c
http://www.pulseaudio.org/browser/trunk/src/pulsecore/flist.h
Comment 3 Wim Taymans 2007-11-30 17:41:10 UTC
it should use the slice allocator in glib and thus save allocs.
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2007-11-30 19:31:30 UTC
Wim, that would not be a good idea. GSlice uses thread local pools und thus requires locking when you relase memory that was allocated in different threads. That unfortunately is the usual case in gstreamer.
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2007-12-03 07:19:34 UTC
Created attachment 100098 [details] [review]
reuse qos event

Now the patch should be save. It uses _make_writeable() and the result in my tests is the same. The qos event is usualy not reffed.

Btw, we don't have
#define         gst_event_make_writable(event)	GST_EVENT (gst_mini_object_make_writable (GST_MINI_OBJECT (event)))

The patch could use that and I can add that one too and adjust the patch.
Comment 6 David Schleef 2007-12-07 23:47:59 UTC
By my calculations, eliminating these mallocs saves about 26 microseconds per second of video processing.  A very unimpressive savings if it is due to adding more than a trivial amount of code.  I recommend using GSlice, too.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2007-12-08 10:07:59 UTC
Okay, some more background. Its not about saving time as such. Its about eliminating non-deterministic behaviour. When calling g_free() which calls free() the libc allocator does trimming of the memory pool. Thus free() is not always quick. When running a low-latency pipeline this could already cause missing deadline. Bug 501239 has a proposal for allowing threads with soft-realtime scheduling. There it is even more important to avoid malloc/free calls.

I already explained why we don't want to use g_slice for most cases in gstreamer in Comment #4. David, do you thing this wont apply?
Comment 8 David Schleef 2007-12-08 15:02:56 UTC
I think you are applying a band-aid to a gaping wound.

This situation reminds me of the bad old days before glib-2.0, when glib and gtk were not thread-safe, LinuxThreads was ass, and doing anything with threading blew planet-sized chunks.  The background level of bugginess was so bad that we could never tell where the bugs were, and added band-aid fixes everywhere.

Now, we have the same situation with time determinism.  Nothing was designed to be deterministic, none of the libraries we use are even remotely deterministic, and we have very few tools to help us down this path.

I recommend starting with a deterministic general-purpose allocator.
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2007-12-08 15:09:11 UTC
(In reply to comment #8)
> 
> I recommend starting with a deterministic general-purpose allocator.
> 
Yes, you're right. That probably the ultimate solution. I though in this case the code-change is little enough to help.

Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2007-12-10 17:58:53 UTC
I've started a tracker Bug 502891. Lets collect some background on this and then discuss how we can improve. Thanks for the feedback.
Comment 11 Sebastian Dröge (slomo) 2009-05-07 13:35:04 UTC
I guess this bug can be closed then... feel free to reopen otherwise.