[pulseaudio-discuss] [PATCH RFCv3 25/51] mainloop: Clear wakeup pipe only when necessary

[David Henningsson]

On 2014-11-05 00:26, Peter Meerwald wrote:
> From: Peter Meerwald <p.meerwald at bct-electronic.com>
>
> when the wakeup pipe became ready, and poll() returned
> just one descriptor, we can stop scanning io_events

Eh, does this really make things faster?

Right now it seems to add another syscall - with the patch, we call 
clear_wakeup -> pa_read -> read syscall twice per iteration, once from 
dispatch_pollfds and once from pa_mainloop_prepare.

In addition, I don't see anything particular time consuming in looping 
through a few io_events.

Replacing the wakeup pipe with a pa_fdsem would be an interesting 
optimisation though, as pa_fdsem has a few atomic operations to avoid 
the entire write-poll-read cycle if no thread is currently waiting for 
the fdsem.

>
> Signed-off-by: Peter Meerwald <pmeerw at pmeerw.net>
> ---
>   src/pulse/mainloop.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/pulse/mainloop.c b/src/pulse/mainloop.c
> index c7a5236..a7a3c48 100644
> --- a/src/pulse/mainloop.c
> +++ b/src/pulse/mainloop.c
> @@ -635,6 +635,15 @@ static void rebuild_pollfds(pa_mainloop *m) {
>       m->rebuild_pollfds = false;
>   }
>
> +static void clear_wakeup(pa_mainloop *m) {
> +    char c[10];
> +
> +    pa_assert(m);
> +
> +    while (pa_read(m->wakeup_pipe[0], &c, sizeof(c), &m->wakeup_pipe_type) == sizeof(c))
> +        ;
> +}
> +
>   static unsigned dispatch_pollfds(pa_mainloop *m) {
>       pa_io_event *e;
>       unsigned r = 0, k;
> @@ -642,6 +651,11 @@ static unsigned dispatch_pollfds(pa_mainloop *m) {
>       pa_assert(m->poll_func_ret > 0);
>
>       k = m->poll_func_ret;
> +    if (m->pollfds[0].revents) {
> +        clear_wakeup(m);
> +        m->pollfds[0].revents = 0;
> +        k--;
> +    }
>
>       PA_LLIST_FOREACH(e, m->io_events) {
>
> @@ -775,20 +789,10 @@ void pa_mainloop_wakeup(pa_mainloop *m) {
>           pa_log("pa_write() failed while trying to wake up the mainloop: %s", pa_cstrerror(errno));
>   }
>
> -static void clear_wakeup(pa_mainloop *m) {
> -    char c[10];
> -
> -    pa_assert(m);
> -
> -    while (pa_read(m->wakeup_pipe[0], &c, sizeof(c), &m->wakeup_pipe_type) == sizeof(c))
> -        ;
> -}
> -
>   int pa_mainloop_prepare(pa_mainloop *m, int timeout) {
>       pa_assert(m);
>       pa_assert(m->state == STATE_PASSIVE);
>
> -    clear_wakeup(m);
>       scan_dead(m);
>
>       if (m->quit)
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic