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

Subject: Re: [boost] Boost.Fiber review January 6-15
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2014-01-07 06:37:32

On 6 Jan 2014 at 8:07, Nat Goodspeed wrote:

> Please always state in your review whether you think the library should be
> accepted as a Boost library!

Currently I am undecided. My biggest issue is in the (lack of) 
documentation. Without very significant improvements in the docs I'd 
have to recommend no right now.

> Additionally please consider giving feedback on the following general
> topics:
> 
> - What is your evaluation of the design?

The design overall is fine. I agree with the decision to replicate 
std::thread closely, including all the support classes.

My only qualm with the design really is I wish there were combined 
fibre/thread primitives i.e. say a future implementation which copes 
with both fibers and threads. I appreciate it probably wouldn't be 
particularly performant, but it sure would ease partially converting 
existing threaded code over to fibers, which is probably a very large 
majority use case.

I accept this can be labelled as a future feature, and shouldn't 
impede entering Boost now.

> - What is your evaluation of the implementation?

The quality of the implementation is generally excellent. My only 
issue is that the Boost.Thread mirror classes do not fully mirror 
those in Boost.Thread. For example, where is 
future::get_exception_ptr()? I think mirroring Boost.Thread rather 
than std::thread is wise for better porting and future proofing.

I'd recommend finishing matching Boost.Thread before entering Boost.

Another suggestion is that the spinlock implementation add memory 
transactions support. You can find a suitable spinlock implementation 
in AFIO at 
https://github.com/BoostGSoC/boost.afio/blob/master/boost/afio/detail/
MemoryTransactions.hpp.

> - What is your evaluation of the documentation?

My biggest concerns with admitting this library now are with the 
documentation:

1. There is no formal reference section. What bits of reference 
section there is does not contain reference documentation for all the 
useful classes (e.g. the ASIO support).

2. I see no real world benchmarks. How are people supposed to know if 
adopting fibers is worthwhile without some real world benchmarks? I 
particularly want to know about time and space scalability as the 
number of execution contexts rises to 10,000 and beyond.

3. I see no listing of compatible architectures, compilers, etc. I 
want to see a list of test targets, ones regularly verified as 
definitely working. I also want to see a list of minimum necessary 
compiler versions.

4. I deeply dislike the documentation simply stating "Synchronization 
between a fiber running on one thread and a fiber running on a 
different thread is an advanced topic.". No it is not advanced, it's 
*exactly* what I need to do. So tell me a great deal more about it!

5. Can I transport fibers across schedulers/threads? swap() suggests 
I might be able to, but isn't clear. If not, why not?

6. What is the thread safety of fiber threading primitives? e.g. if a 
fibre waits on a fibre future on thread A, can thread B signal that 
fibre future safely? If not, what will it take to make this work, 
because again this is something I will badly need in AFIO.

7. I want to see worked tutorial examples in the documentation e.g. 
step me through implementing a fibre pool, and then show me how to 
implement a M:N threading solution. That sort of thing, because this 
is another majority use case.

8. There are quite a few useful code examples in the distribution in 
the examples directory not mentioned in the formal docs (which is 
bad, because people don't realise they are there), but they don't 
explain to me why and how they work. I find the ASIO examples 
particularly confusing, and I don't understand without delving into 
the implementation code why they work which is a documentation 
failure. This is bad, and it needs fixing.

9. It would help a lot to understand missing features if there were a 
Design Rationale page explaining how and why the library looks the 
way it does.

> - What is your evaluation of the potential usefulness of the library?

It's very useful, and I intend to add fiber support to proposed 
Boost.AFIO.

> - Did you try to use the library? With what compiler? Did you have any
> problems?

Not yet. AFIO needs to be modified first (targeted thread_source 
support).

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

A few hours as I decided exactly how AFIO will add Fiber support. I 
reviewed 
both the source distribution and the docs.

> - Are you knowledgeable about the problem domain?

Yes. I wrote one of these myself in assembler in the 1990s.

Niall

-- 
Currently unemployed and looking for work.
Work Portfolio: http://careers.stackoverflow.com/nialldouglas/