Re: [boost] Boost.Fiber review January 6-15

Subject: Re: [boost] Boost.Fiber review January 6-15
From: Eugene Yakubovich (eyakubovich_at_[hidden])
Date: 2014-01-08 17:37:19

On Mon, Jan 6, 2014 at 7:07 AM, Nat Goodspeed <nat_at_[hidden]> wrote:
>
> Please always state in your review whether you think the library should be
> accepted as a Boost library!
I think the library should be accepted but would prefer some changes made as
outlined below.

>
> Additionally please consider giving feedback on the following general
> topics:
>
> - What is your evaluation of the design?
The overall design is good. I like the parallel between thread interface
and fiber interface.

Some suggestions:
- Maybe this is something that should be handled in a separate fiber
pool library but I'd like to be able to specify multiple threads for
affinity. This is useful for when a fiber should be bound to any one
of threads local to a NUMA domain or physical CPU (for cache sharing).

- I dislike fiber_group. I understand that it parallels thread_group.
I don't know the whole story behind thread_group but I think it
predates move support and quick search through the mailing list
archives shows there were objections about its design as well. I
specifically don't like having to new up fiber object to pass
ownership to add_fiber. fiber object is just a handle (holds a
pointer) so it's a great candidate for move semantics. It maybe best
to take out fiber_group altogether. What's a use case for it?

> - What is your evaluation of the implementation?
- Can I suggest replacing the use of std::auto_ptr with
boost::scoped_ptr? It leads to deprecation warnings on GCC.

- While I understand that scheduling algorithm is more internal than
the rest of the library, I still don't like detail namespace leaking
out. Perhaps these classes should be moved out of the detail
namespace.

- algorithm interface seems to do too much. I think a scheduling
algorithm is something that just manages the run queue -- selects
which fiber to run next (e.g. Linux kernel scheduler interface works
like this). As a result, implemented scheduling algorithms have much
overlap. Indeed, round_robin and round_robin_ws is almost identical
code.

> - What is your evaluation of the documentation?
Ok but, like others have said, could be improved. ASIO integration is
poorly documented.

> - What is your evaluation of the potential usefulness of the library?
Very useful. I've used Python's greenlets (via gevent) and they were
very useful for doing
concurrent I/O. Would love to see equivalent functionality in C++.

> - Did you try to use the library? With what compiler? Did you have any
> problems?
Yes, I used it but very little. Used g++ 4.8.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
Spend about 3 hours studying the code and writing toy examples.

> - Are you knowledgeable about the problem domain?
Not really but I've spent many years writing server code with
completion routines so I know the value of not having to do that.