Please always state in your review whether you think the library should be accepted as a Boost library!
Library should be accepted, but some modification must be applied (see below).
Additionally please consider giving feedback on the following general topics:
- What is your evaluation of the design?
Part that mimics the Boost.Thread interface is good. Missing functions and minor updates can be easily added even after the review. Scheduler does not look good. Class "algorithm" looks overcomplicated: too many methods, classes from detail namespace are used in function signatures, looks like some methods duplicate each other (yield, wait). I'd recommend to hide "algorithm" from user for nearest releases in detail namespace, refactor it and only after that - show it to user. Maybe an additional mini review will be required for algorithm.
- What is your evaluation of the implementation?
I'm slightly confused by the fact that fiber_base use many dynamic memory allocations (it contains std::vector, std::map) and virtual functions. There must be some way to optimize away that, otherwise fibers may be much slower than threads. Heap allocations also exist in mutex class. I dislike the `waiting_. push_back( n);` inside the spinlocked sections. Some intrusive container must be used instead of std::deque. There is a copy-pasted code in mutex.cpp. `try_lock` and `lock` member functions have very different implementations, that's suspicious. Back to the fiber_base. There's too many atomics and locks in it. As I understand, the default use-case does not include thread migrations so there is no need to put all the synchronizations inside the fiber_base. round_robin_ws and mutexes work with multithreading, ideologically it's the more correct place for synchronizations. Let fiber_base be thread unsafe, and schedulers and mutexes take care about the serializing access to the fiber_base. Otherwise singlethreaded users will loose performance.
- What is your evaluation of the documentation?
Please add "Examples" section. Paul Bristow already mentioned a QuickBook's ability to import source files using [@../../example/my_example.cpp] syntax.
- What is your evaluation of the potential usefulness of the library?
I have a few complicated libraries that will benefit from fibers. However short real-life usecase examples will be good to get the idea of fibers to new library users.
- Did you try to use the library? With what compiler? Did you have any problems?
There were no problems with GCC-4.7 and clang. Many warnings were rised during compilation, but most part of the warnings belong to the DateTime library.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I've tried a few examples, made a bunch of experiments with asio. Spend few hours looking through the source code.
- Are you knowledgeable about the problem domain?
Not a pro with fibers, but have good experience with threads. -- Best regards, Antony Polukhin