[RFC] extending ev++.h to add C++ loop wrappers.

Marc Lehmann schmorp at schmorp.de
Tue Jan 15 08:06:37 CET 2008


On Mon, Jan 14, 2008 at 01:37:36PM -0200, Leandro Lucarella <llucax at gmail.com> wrote:
> Hi, following the "C++ APIs" thread [1], I came to the conclusion that I
> didn't like the "whole new wrapper" approach, because it didn't provide
> anything ev++.h have

Well, it would, according to the original plan :)

> (or it would be too hard to implement, and I didn't
> thought it worth the effort, because there were too little benefit).

Thats something else altogether :)

Now, wouldn't it be nice if all the ev watchers had some type field, which
could then just be a vtbl pointer for C... :->

> So I decided to improve ev++.h, at least as I see it =)

Thats fine!

> your opinion before deciding what to do with them, and of course, I want
> to know if there is any interest in merging this to the official libev
> distribution =)

Yes, there is. Two things are required for inclusion:

a) it should be sound (i.e. required, within the goals, i.e. relatively lean)
b) somebody needs to maintain it

a) is probably given (I haven't had a thorough look :), and would you
commit to do b)?

> 1) Storing a ev_loop* in the watchers, instead of a loop_ref. But I'm
>    affraid that not doing so could break some existing code.

It certainly does break existing code :)

See my comments on the diff later.

> 2) The default loop handling. Now a default loop has to be instantiated,
>    and a simple check is done to ensure just one default loop exists, but
>    with an ugly hack (via de default_loop_created() function) to avoid the
>    need of object code (a class static variable would need it).

ev.h *could* probably export the address of the default loop, but ti still
needs initialising.

The only way I see to do that form a header file is the ugly way we do it
now.

>    explicitly free the default loop). Now that I see this, maybe it could
>    be "elegantly" solved using a reference:

Indeed. But have a look at the generated code :)

I don't think there is a ncie solution, however. The nicest way with this
method would be to not punish programs not using the loop wrapper (see
again my comments below).

Another option would be not to have _a_ default loop at all, but many, i.e.

   default_loop&
   get_default_loop(int flags = 0)
   {
     return default_loop (ev_default_loop (flags));
   }

As long as loop_base has the same size as a pointer, this approach should
be quite efficient: instead of handing around naked struct ev_loop *'s you
basically hand around augmented nice pointers that atcually behave like
nice objects.

> To implement this point, 2) is necessary if we don't want to be needed to
> explicitly set every watcher's loop, because the lack of a "standard" way
> to get the default loop.

Actually, I really dislike the many calls to ev_default_loop in current
ev++.h. OTOH, the user can always avoid them nicely, and at one point one has
to set the loop, so I guess its fine.

> Here's patch that implement this scheme (over the original patch). Even if
> it's nice to stay close to the C API and to avoid the posibility of
> instantiating 2 default loops from the root, I think the usage is a little
> less intuitive in a C++ environment.

libev already enforces the singleton approach for us, the loop_default
objects can exist manyfold without us having to worry about it.

On Mon, Jan 14, 2008 at 06:40:08PM -0200, Leandro Lucarella <llucax at gmail.com> wrote:
> * is_default was removed from loop_base, since now the class matches the
>   type of loop (is_default == dynamic_cast< loop_default* >(loop_ptr)).

Hmm, the loops are not polymorphic, so the dynamic_cast will not be helpful.
In any case, is_default == (loop_ == ev_default_loop (0)).

> watcher) are attached. Again, this is a rough patch, if you like it,
> cleaning and documentation will follow =)

I like it, with comments :->

Here are my comments on the latest diff:

> +  enum
> +  {
> +    AUTO = EVFLAG_AUTO,
> +    NOENV = EVFLAG_NOENV,
> +    FORKCHECK = EVFLAG_FORKCHECK,

putting all the constants into the ev:: namespace was my original goal
(when I hoped to keep most of the C api outside the global namespace), but
I gave up on this.

In any case, I think there should either be ALL of the symbols available or
none (or they shouldn't be announced), as remembering whats in ev:: and whats
not is tedious.

That was the reason I originally kept out most of the functions outside it,
too (ev::now is the exception, and it doesn't look well).

If everything *is* inside ev:: that would be fine, but its quite a bit of
work. Otherwise, I have no hard feelings either way.

> +  enum how_t
> +  {
> +    ONE = EVUNLOOP_ONE,
> +    ALL = EVUNLOOP_ALL

Thats different, this way we do gain a bit of type-safety :)

> +  struct bad_loop: std::runtime_error
> +  {
> +    bad_loop()
> +      : std::runtime_error("loop can't be initialized") {}
> +  };


> +#if EV_MULTIPLICITY
> +#  define EV_AX  _loop
> +#  define EV_AX_ _loop,
> +#else
> +#  define EV_AX
> +#  define EV_AX_ ,

Both EV_AX and EV_AX_ should be empty here, no?

> +#if EV_MULTIPLICITY
> +    operator struct ev_loop * () const throw ()
> +    {
> +      return _loop;

for various reasons, I would prefer the "_" at the end, but again, I don't
care much :)

I wonder why it cannot be simply "loop"? Does it collide with anything?

> +    void unloop (how_t how = ALL) throw ()

I think the default should be "ONE", if only because the perl interface
also defaults it to "ONE" (alternatively, if there is a good reason to use
ALL (I think ONE is atcually better), then both should be changed to ALL).

> +    virtual void fork () throw () = 0;

fork () might be a global symbol that would collide with this declaration
(no idea if it atcually causes a problem on existing systems, though).

> +    void set_io_collect_interval (tstamp interval) throw ()
> +    void set_timeout_collect_interval (tstamp interval) throw ()

Looks quite complete already :)

> +    // method callback
> +    template<class K, void (K::*method)(int)>
> +    void once (int fd, int events, tstamp timeout, K *object)

pure fun!

> +    // simpler function callback
> +    template<void (*cb)(int)>
> +    void once (int fd, int events, tstamp timeout)
> +    {
> +      once (fd, events, timeout, simpler_func_thunk<cb>);
> +    }

Fascinating, shouldn't we add this to the watcher base, too?

I do wonder how useful it is, I wondered about it myself, but I tend to
always need the watcher in any real-world callback.

> +    ~loop_simple () throw ()

Maybe loop_simpel should rarlly be "loop" if we can get away with it
without collisions.

> +  inline
> +  loop_default&
> +  default_loop (int flags)
> +  {
> +    static loop_default l (flags);
> +    return l;
> +  }

I my, I am scared at how horrible code that will generate in each
translation unit (I know, the same thing is done in ev.h, but its still
ugly :). Maybe leave it up to the user to declare her own loop_default
object? Or maybe not.

>    template<class ev_watcher, class watcher>
>    struct base : ev_watcher
>    {
>      #if EV_MULTIPLICITY
> -      EV_P;
> +      loop_base *_loop;

Thats the only negative thing, as it creates a need to use the ev++.h loop
wrappers.

I'd really prefer using the ev_loop *, even if it means slightly more
annoying code in the watcher.

Alternatively, how about storing a loop_base in the watcher (note: no "*")
and accepting both ev_loop * and ev::loop_bases as arguments to set and the
constructor?

In fact, since loop_base is comaptible to struct ev_loop one could just
support ev_loop *'s and internally store a loop_base.

One would lose the ability to compare loop_base's by address, but a nice
operator == would do that or somesuch, if needed.

> +  inline void sleep (tstamp interval)

Sleep, again, could collide with a global symbol defined in a header,
unfortunately.

> -    void start (int fd, int events)
> +    void start (int fd, int events = READ)

Do you really think this is such a good idea? Is READ really predominantly
what people want?

Summary:

all in all, this is surprisingly straightforward and thin. its really in the
spirit of ev++.h, I think (or what I define to be that spirit :).

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      pcg at goof.com
      -=====/_/_//_/\_,_/ /_/\_\



More information about the libev mailing list