Proposal: Add newTimeoutKey and insertTimeout to System.Event

[Johan Tibell]

Hi,

You might want to read
http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#code_ev_timer_code_relative_and_opti
. The current design is based on a subset of libev's API (i.e. we
don't support resettable or periodic timers at the moment).

On Wed, Mar 16, 2011 at 6:09 PM, Bas van Dijk <v.dijk.bas at gmail.com> wrote:
> Hello,
>
> I would like to propose adding and exporting the following functions
> from System.Event.Manager and also export them from System.Event:
>
> -- | Returns a unique 'TimeoutKey' that can be used by 'insertTimeout'.
> newTimeoutKey :: EventManager -> IO TimeoutKey
> newTimeoutKey = fmap TK . newUnique . emUniqueSource

I'm worried that if we expose this function in the API we'd have a
hard time to switch the implementation to e.g. a mutable one that no
longer uses Uniques. While we can manufacture some kind of token (e.g.
a pointer) to represent a position in a mutable data structure it
might not be possible to do so without actually inserting something
into the data structure.

Perhaps it's possible though. libev has a separate type representing
timers themselves, rather than a TimeoutKey like we use.

It seems like your use case can be implemented in terms of just
registerTimeout and updateTimeout, so do you really need this
function?

> -- | Like 'registerTimeout' but registers the timeout in the given number of
> -- microseconds /at the specified key/ in the event manager. If the key was
> -- already present the associated timeout is replaced with the given timeout.
> -- Unique keys can be created using 'newTimeoutKey'.
> insertTimeout :: EventManager -> TimeoutKey -> Int -> TimeoutCallback -> IO ()

I'd prefer to call this 'updateTimeout' as "insert" sounds more like
an implementation detail.

> The reason I would like to have these is that with them I can write
> more efficient resettable timeouts. A resettable timeout API looks
> something like this:
>
> data Key
> register :: EventManager -> Int -> TimeoutCallback -> IO Key
> pause :: Key -> IO ()
> reset :: Key -> IO ()
> cancel :: Key -> IO ()
>
> 'register mgr us cb' registers a callback 'cb' with the event manager
> 'mgr' to fire after 'us' microseconds. It returns a key which can be
> used to control the timeout. When 'pause' is called on a key,
> belonging to a timeout which hasn't fired yet, the timeout is
> suspended indefinitely until it is reset or canceled. When 'reset' is
> called on a key, belonging to a timeout which hasn't fired yet, the
> timeout period will be extended with at least the original period.
> 'cancel' removes the timeout from the event manager.

Doesn't this mean that we'll have to store the period somewhere,
together with the key?

> The Warp web-server contains one implementation of resettable timeouts:
>
> https://github.com/snoyberg/warp/blob/master/Timeout.hs
>
> Note that their API is a bit more restricted but the idea is the same.
> The implementation uses one thread which runs a loop which delays for
> the timeout period then processes all the registered timeouts.
> Registered timeouts are put in a list and stored in an IORef.
>
> With the proposed additional functions for System.Event I can write a
> simpler (and hopefully slightly more efficient) implementation of
> resettable timeouts which don't need to forkIO and don't need to store
> timeouts in a list:

I think it's important to figure out if this scheme is actually faster
than using register/unregister. If the only gain is a bit of
convenience in user code I'd be wary of adding complexity to the I/O
manager itself, as it involves changes to base, which can only be
release so often.

> Discussion deadline: 2 weeks.

Note that the I/O manager is not managed under the libraries process,
at least not yet. System.Event is still considered a GHC internal API
(we never proposed it using the libraries process in the first place).
It should probably have been renamed to GHC.Event until it was
proposed but that never happened. I sent a patch to the GHC mailing
list that updated the documentation to state that this API should be
considered internal, but the patch was most likely lost somewhere
along the road.

Johan

P.S. I'm in Thailand so I'm a bit slow in replying to emails.