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 16:32:13

On 7 Jan 2014 at 19:35, Oliver Kowalke wrote:

> I'll add an rational section to the documentation - the library is
> pre-alpha (e.g. I'm experimenting with some ideas - but if you are
> interested I can discuss it with you in a private email). 

I know the feeling. Bjorn (Reese) has been doing a sort of private 
peer review of AFIO with me recently, and I keep telling him things 
are weird in AFIO because it's all very alpha and I want to keep my 
future design options open.

Thing is, Bjorn is generally right, and bits of AFIO suck and need 
fixing. I've been trying my best to fix things, but it ain't easy 
compiling and debugging Boost on an Intel Atom 220 (all I have 
available right now).

> This is one items which could be discussed. for instance I've concerns
> to add future::get_exception_ptr() because I believe it is not really
> required, it is only for convenience. 

Ehhh not really ... it lets you avoid a catch(), which is one of the 
few places in C++ without a worst case complexity guarantee. Catching 
types with RTTI (anything deriving from std::exception) in a large 
application can be unpleasantly slow (and unpredictably slow), and if 
you're bouncing exception state across say five separate futures, 
that's five separate unnecessary try...catch() invocations.

BTW, older compilers also tend to serialise parts of try...catch() 
processing across threads. If you fire off a thousand threads doing 
nothing but throwing and catching exceptions you'll see some fun on 
older compilers (e.g. pre-VS2012).

> > I don't understand what they do, and I think I ought to.
> 
> It is pretty simple - the fiber-scheduler contains a reference to asio's
> io_service and uses it a event-queue/event-dispatcher, e.g. it pushes
> fibers ready to run (e.g. fiber newly created, yielded fibers or signaled
> fibers suspended by a wait-operation on sync. primitives). 

Oh I see ... just like AFIO does with ASIO's io_service in fact.

If it's that easy, adding Fiber support should be a real cinch.

> I don't know the factor of fibers scaling - I would consider boost.fiber
> a way to simplify the code, e.g. it prevents scattering the code by
> callbacks (for instance in the context of boost.asio). my starting point
> was to solve problmes like the C10K-problem (Dan Kegel explains it in a
> more detail on its webpage - I'm referring to it in boost.fiber's
> documentation -
> http://olk.github.io/libs/fiber/doc/html/fiber/asio.html). 

Thing is ... the C10K problem *is* a performance problem. If you're 
going to suggest that Boost.Fiber helps to solve or does solve the 
C10K problem, I think you need to demonstrate at least a 100K socket 
processing capacity seeing as some useful work also needs to be done 
with the C10K problem. Otherwise best not mention the C10K problem, 
unless you're saying that you hope in the near future to be able to 
address that problem in which case the wording needs to be clarified.

> well that are the problems we are all faced to  :)  I've not done
> performance tests, sorry

I appreciate and understand this. However, I must then ask this: is 
your library ready to enter Boost if you have not done any 
performance testing or tuning?

> > Eh, well then I guess you need a link to the correct page in
> > boost.context where it lists the architectures it works on. Certainly
> > a big question for anyone considering Fiber is surely "will it work
> > on my CPU"?
> 
> the docu of boost.context is mising this info - I'll add it (could you add
> an bug-report for boost.context please).

https://svn.boost.org/trac/boost/ticket/9551

> > and we'll figure out the rest. I definitely need the ability to
> > signal a fibre future belonging to thread A from some arbitrary
> > thread B. I'll also need to boost::asio::io_service::post() to an
> > ASIO io_service running fibers in thread A from some arbitrary thread
> > B.
>
> yes, as I explained it is supported.Maybe because I've writen the code
> don't know why the documentation is not enought for you. 

It wasn't clear from the docs that your Fiber support primitives 
library is also threadsafe. I had assumed (like Antony seemed to as 
well) that for speed you wouldn't have done that, but it certainly is 
a lot easier if it's all threadsafe too.

> what works is that you create fibers::packaged_task<> and execute it in
> a fiber on thread A and wait on fibers::future<> returned by
> packaged_task<>::get_future() in thread B. 

Yes I understand now. fibers::packaged_task<> is a strict superset of 
std::packaged_task<>, not an incommensurate alternative. That works 
for me, but others may have issue with it.

> > The docs need to explicitly say so then, and indeed thread safety for
> > *every* API in the library.
> 
> OK

BTW, I did this in AFIO by creating a macro with the appropriate 
boilerplate text saying "this function is threadsafe and exception 
safe", and then doing a large scale regex find and replace :) I only 
wish I could do the same for complexity guarantees, but that requires 
studying each API's possible code paths individually.

> > I think, as a minimum, all the examples need to appear verbatim 
in
> > the docs
> 
> the complete code? I would prefer only code snippets - the complete code
> be read in the example directory. otherwise the documentation would be
> bloated (at least I would skip pages of code). 

You can stick them into an Appendix section at the bottom. They're 
purely there for Googlebot to find, no one is expecting humans to 
really go there.

That said, if you want to do an annotated commentary on a broken up 
set of snippets from the examples, do feel free :). But I think 
inline source comments is usually plenty enough.

You asked about the comments you added to 
https://github.com/olk/boost-fiber/blob/master/examples/asio/publish_s
ubscribe/server.cpp. They do help, but I am still struggling 
somewhat.

How about this? Can you do a side-by-side code example where on the 
left side is an old style ASIO callback based implementation, and on 
the right is an ASIO Fiber based implementation? Something like 
https://ci.nedprod.com/job/Boost.AFIO%20Build%20Documentation/Boost.AF
IO_Documentation/doc/html/afio/quickstart/async_file_io/hello_world.ht
ml.

It doesn't have to be anything more than a trivial Hello World style 
toy thing. I just need to map in my head what yield[ec] means, and 
how that interplays with boost::fibers::asio::spawn and 
io_service::post().

Also one last point: your fiber condvar seems to suffer from no 
spurious wakeups like pthread condvars? You're certainly not 
protecting it from spurious wakeups in the example code. If spurious 
wakeups can't happen, you *definitely* need to mention that in the 
docs as that is a much tighter guarantee over standard condvars.

Niall

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