Re: current_time in main loop

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj@gtk.org> writes:

> hey owen,
> 
> i just came across the current_time values you pass in to glib main loop
> sources in prepare(), check() and dispatch().
> while current_time contains a reasonably valid value for prepare() (minus
> the delay that occoured from previoud prepare() calls), it is more or
> less bogus (too old) for check() and dispatch():
 
[...]

> at least directly after poll() we should refetch current_time, and since
> dispatching can be quite time consuming for some sources, i think we should
> also refetch it after a source has been dispatched.
> with what we currently have, all current_time values only refer to the one
> moment when we started out with preparing the sources, and i don't really
> see how that would be usefull (in check() and dispatch()).
> if, for *some* reason that is not yet aparent to me, you actually mean to
> pass around time_when_loop_iteration_started, then we have to at least
> rename current_time to something different.

Hi Tim,

I've had a chance to look at this now, and I'd agree that we need to
refetch the time at the end of the poll(). It tends to actually work
OK for a lot of things now, because usually we'll go around to the
next iteration immediately, and the timeout will be dispatched without
further waiting; but the intervals get rather screwed up, because the
time for the next iterations is _after_ all the other dispatches in
the same batch get fired, so the next timout is potentially delayed
under load. 

The tests you did with BEAST tend to be empirical confirmation that
the change helps.

But I'm less sure we should refetch the time after each dispatch.
Firt, lets consider a real world example:

 0ms:  timers start
 30ms: timer 1 fires, expiration = 60ms
 40ms: timer 2 fires, expiration = 70ms
 60ms: timer 1 fires, expiration = 90ms
 70ms: timer 2 fires, expiration = 100ms

If we refetch only after the poll, then we have:

 0ms:  timers start
 30ms: timer 1 fires, expiration = 60ms
 40ms: timer 2 fires, expiration = 60ms
 60ms: timer 1 fires, expiration = 90ms
 70ms: timer 2 fires, expiration = 90ms

Not too much to choose between here - either way, we
get the right intervals, though with the first, there
is some chance of slippage, since the delay of the
second timer is permeanent, even if the first timer
removed.

So, for pure performance, I'm not really sure what 
difference it makes. I suppose we need a mainloop
torture test, and do some testing.

But there are a couple of, I think, good reasons to 
not update the current time after the dispatch:

 - It doesn't fit into the "theory" of the main loop, which
   is that after the poll, we effectively take a "snapshot"
   of the current state by calling check on each source, then dispatch
   them. This means, for instance, that even if we do get
   a newer current time, then we can't decide to dispatch another
   timeout based on it. The reason we do things this way
   is to get a consistent view of the priorities of the sources.

 - If we don't know whether we need to refetch it, then why don't we
   save the 1.6us and not do so. Syscalls are cheap on most OS's, but
   getting the time of day can be a fairly involved process. (On
   intel, it involves blocking interrupts, obtaining a lock in the
   kernel, etc.)

Maybe you want to retry your tests with BEAST and see if you
get the same improvements with only refetching after the poll().

(And, as I noted, we only want to refetch the time if we poll()
 with a non-zero timeout.)

Regards,
                                        Owen