On Fri, 2016-03-04 at 09:45 +0530, Arun Raghavan wrote:
> On Thu, 2016-03-03 at 18:02 +0200, Tanu Kaskinen wrote:
> > On Tue, 2016-02-09 at 12:42 +0200, Tanu Kaskinen wrote:
> > >
> > > On Tue, 2016-02-09 at 15:47 +0530, Arun Raghavan wrote:
> > > >
> > > > On Fri, 2015-01-02 at 15:04 +0200, Tanu Kaskinen wrote:
> > > > >
> > > > > +}
> > > > > +
> > > > > +static void find_expired_time_events(pa_rtpoll *rtpoll) {
> > > > > +Â Â Â Â pa_usec_t now;
> > > > > +Â Â Â Â pa_time_event *event;
> > > > > +Â Â Â Â unsigned idx;
> > > > > +
> > > > > +Â Â Â Â pa_assert(rtpoll);
> > > > > +Â Â Â Â pa_assert(pa_dynarray_size(rtpoll->expired_time_events) == 0);
> > > > > +
> > > > > +Â Â Â Â now = pa_rtclock_now();
> > > > > +
> > > > > +Â Â Â Â PA_DYNARRAY_FOREACH(event, rtpoll->enabled_time_events, idx) {
> > > > > +Â Â Â Â Â Â Â Â if (event->time <= now)
> > > > > +Â Â Â Â Â Â Â Â Â Â Â Â pa_dynarray_append(rtpoll->expired_time_events, event);
> > > > > +Â Â Â Â }
> > > > > +}
> > > > > +
> > > > > +static pa_time_event *find_next_time_event(pa_rtpoll *rtpoll) {
> > > > > +Â Â Â Â pa_time_event *event;
> > > > > +Â Â Â Â pa_time_event *result = NULL;
> > > > > +Â Â Â Â unsigned idx;
> > > > > +
> > > > > +Â Â Â Â pa_assert(rtpoll);
> > > > > +
> > > > > +Â Â Â Â if (rtpoll->cached_next_time_event)
> > > > > +Â Â Â Â Â Â Â Â return rtpoll->cached_next_time_event;
> > > > > +
> > > > > +Â Â Â Â PA_DYNARRAY_FOREACH(event, rtpoll->enabled_time_events, idx) {
> > > > > +Â Â Â Â Â Â Â Â if (!result || event->time < result->time)
> > > > > +Â Â Â Â Â Â Â Â Â Â Â Â result = event;
> > > > > +Â Â Â Â }
> > > > > +
> > > > > +Â Â Â Â rtpoll->cached_next_time_event = result;
> > > > > +
> > > > > +Â Â Â Â return result;
> > > > > +}
> > > > > +
> > > > > Â static void reset_revents(pa_rtpoll_item *i) {
> > > > > Â Â Â Â Â struct pollfd *f;
> > > > > Â Â Â Â Â unsigned n;
> > > > > @@ -204,9 +632,14 @@ static void reset_all_revents(pa_rtpoll *p) {
> > > > > Â }
> > > > > Â
> > > > > Â int pa_rtpoll_run(pa_rtpoll *p) {
> > > > > +Â Â Â Â pa_defer_event *defer_event;
> > > > > +Â Â Â Â pa_time_event *time_event;
> > > > > Â Â Â Â Â pa_rtpoll_item *i;
> > > > > Â Â Â Â Â int r = 0;
> > > > > Â Â Â Â Â struct timeval timeout;
> > > > > +Â Â Â Â pa_time_event *next_time_event;
> > > > > +Â Â Â Â struct timeval next_time_event_elapse;
> > > > > +Â Â Â Â bool timer_enabled;
> > > > > Â
> > > > > Â Â Â Â Â pa_assert(p);
> > > > > Â Â Â Â Â pa_assert(!p->running);
> > > > > @@ -218,7 +651,28 @@ int pa_rtpoll_run(pa_rtpoll *p) {
> > > > > Â Â Â Â Â p->running = true;
> > > > > Â Â Â Â Â p->timer_elapsed = false;
> > > > > Â
> > > > > -Â Â Â Â /* First, let's do some work */
> > > > > +Â Â Â Â /* Dispatch all enabled defer events. */
> > > > > +Â Â Â Â while ((defer_event = pa_dynarray_last(p-
> > > > > >
> > > > > > enabled_defer_events))) {
> > > > > +Â Â Â Â Â Â Â Â if (p->quit)
> > > > > +Â Â Â Â Â Â Â Â Â Â Â Â break;
> > > > > +
> > > > > +Â Â Â Â Â Â Â Â defer_event->callback(&p->mainloop_api, defer_event,
> > > > > defer_event->userdata);
> > > > > +Â Â Â Â }
> > > > Am I missing something, or is this an infinite loop if there are any
> > > > enabled defer events?
> > > As discussed in IRC, I did this like this, because pa_mainloop_run() in
> > > practice behaves the same way. However, mainloop-api.h says that each
> > > defer event runs only once per mainloop iteration, so I'll have to
> > > change this.
> > I started looking into this, and realized that the "once per mainloop
> > iteration" clause makes no difference to pa_mainloop_api users. There
> > is no pa_rtpoll_iterate(), and pa_rtpoll_run() will behave the same way
> > in any case, so is it really worth the effort to artificially refactor
> > the code to have "iterations"?
>
> But there is a difference of order, right? The mainloop API idea of an
> iteration is not just about pa_mainloop_iterate(), but also about
> something like:
>
> Â 1. Run all I/O events
> Â 2. Run all expired time events
> Â 3. Run all defer events
> Â 4. Goto 1
>
> The order is not something we commit to, but the fact that we do one
> run of each is. Your code runs a loop in (3) until the defer event
> basically goes away.
That's not how pa_mainloop works. If there are any defer events enabled
when pa_mainloop_iterate() is called, one of each is run, and none of
time and IO events. In practice this means that pa_mainloop_run()
processes defer events as long as there are any enabled, and only then
it processes each expired time and IO event once.
> From what I can tell, this should not required a radical restructuring.
> Instead of always picking pa_dynarray_last(), you could just iterate
> over all the enabled events once.
That would mean processing time and IO events more aggressively than
what pa_mainloop does, and I tried to avoid differing from pa_mainloop
behaviour.
--Â
Tanu
