GNOME Bugzilla – Bug 736782
compositor: performance degrades over time
Last modified: 2015-05-12 08:07:17 UTC
Comparing the following two pipelines: gst-launch-1.0 videotestsrc ! videomixer ! progressreport ! fakesink gst-launch-1.0 videotestsrc ! compositor ! progressreport ! fakesink The one using compositor both grows in memory use and slows down over time. It ends up spending most of its time in g_source_iter_next.
Looks like some g_source/idle callback leak.
Created attachment 286388 [details] [review] patch Here's a patch that fixes the bug. "make check" still passes.
Oh, and just to repeat the patch description here because I forgot: aggregator: Replace GMainContext with GAsyncQueue The previous implementation kept accumulating GSources, slowing down the iteration and leaking memory. Instead of trying to fix the mcontext flushing, replace it with a GAsyncQueue which is simple to flush and has less overhead.
Review of attachment 286388 [details] [review]: Apart from the question it looks good. I will run my testsuite on it. ::: gst-libs/gst/base/gstaggregator.c @@ -486,1 +469,0 @@ - /* Clean the stack of GSource set on the MainContext */ Why don't you flush here directly?
(In reply to comment #4) > ::: gst-libs/gst/base/gstaggregator.c > @@ -486,1 +469,0 @@ > - /* Clean the stack of GSource set on the MainContext */ > > Why don't you flush here directly? There needs to be one item on the queue so that gst_async_queue_pop is unblocked after priv->running is set to FALSE. I suppose I could do "QUEUE_FLUSH (self); QUEUE_PUSH (self);" but that looks weirder to me than pushing one item after running=FALSE, and then flushing the queue once the src task is stopped.
(In reply to comment #5) > (In reply to comment #4) > > ::: gst-libs/gst/base/gstaggregator.c > > @@ -486,1 +469,0 @@ > > - /* Clean the stack of GSource set on the MainContext */ > > > > Why don't you flush here directly? > > There needs to be one item on the queue so that gst_async_queue_pop is > unblocked after priv->running is set to FALSE. I suppose I could do > "QUEUE_FLUSH (self); QUEUE_PUSH (self);" but that looks weirder to me than > pushing one item after running=FALSE, and then flushing the queue once the src > task is stopped. Got it, it makes sense then yes. I will check a few things and push that I think. Thanks for your patch.
Created attachment 287805 [details] [review] rebased patch Rebased the patch to current -bad master, as it conflicted with 1b4547f.
commit dce92c75b1062427605d25d1bc3ff6bb8818d7c4 Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com> Date: Wed Sep 17 16:48:02 2014 +0200 aggregator: Replace GMainContext with GAsyncQueue (v2) The previous implementation kept accumulating GSources, slowing down the iteration and leaking memory. Instead of trying to fix the main context flushing, replace it with a GAsyncQueue which is simple to flush and has less overhead. https://bugzilla.gnome.org/show_bug.cgi?id=736782
Thanks a lot for your patch! :)