> > 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? probably not in a measureable way; I think the patch makes the codes more logical: * if we know that no io_event triggered, there is no point in scanning them (I had 23 io_events for a simple PA setup, not sure where they are all coming from) * if we know that the wakeup pipe triggered, we clear it (currently the wakeup pipe is cleared always, no matter what; this should save a read() when nothing is in the wakeup pipe) > 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. I don't think so; the patch remove the clearing from prepare and only does it in dispatch_pollfds when necessary > In addition, I don't see anything particular time consuming in looping > through a few io_events. probably, yet it's pointless and doesn't convey the idea of the polling framework > 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. though about it as well; I micro-benchmarked pipe() vs. eventfd() write-poll-read and there was only a slight benefit for eventfd since the fdsem would be used in poll, there would be almost-always someone waiting? regards, p. > > 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) > > > > -- Peter Meerwald +43-664-2444418 (mobile)