Hi all, we will start a new year of Boost reviews with a one week mini-review of the Boost.Context library of Oliver Kowalke. The review period starts today January 2nd and ends January 11. Please use [context review] as prefix of your post. Reviewers are strongly encouraged to send their reviews to the boost development mailing list as this is the I follow more assiduously. Please note that Boost.Context was already reviewed and conditionally accepted last May, so we are not voting for acceptance. Instead, this mini-review is to discuss whether the library meets the conditions on acceptance that were listed by Vicente Botet, the review manager at that time. You'll find the whole list at the end of this email. Since the end of the original review, Oliver has spent a large amount of time on a partial rewrite of the library and both me and Oliver decided to schedule this review only after we determined that all issues had been addressed or at least considered. In your review please state whether the acceptance conditions have been addressed satisfactorily. ---------------------------------------------------------------------------------- About the library: Boost.Context is a foundational library that provides a sort of cooperative multitasking on a single thread. By providing an abstraction of the current execution state in the current thread, including the stack (with local variables) and stack pointer, all registers and CPU flags, and the instruction pointer, a context instance represents a specific point in the application's execution path. This is useful for building higher-level abstractions, like coroutines and fibers. ---------------------------------------------------------------------------------- Useful Links: documentation: http://ok73.ok.funpic.de/boost/libs/context/doc/html/ library: git://github.com/olk/boost.context.git ----------------------------------------------------------------------------------

From the original review result email:

Point to be addressed before the mini review: Design 1. Separation of the context class in two interface: - A C-like interface that defines a POD struct that is memcpy-able and that can be implemented either using platform libraries, as ucontext, fibers or directly using assembler when a robust implementation is available. The context swap implementation don't need to save the signals as ucontext does. The following is just for illustrations purposes. struct boost_context; typedef void boost_context_fn_t(intptr_t); extern "C" void boost_context_make(boost_context* ctx, boost_context_fn_t * f, intptr_t p, ...); extern "C" void boost_context_swap(boost_context* from, boost_context const* to); extern "C" intptr_t boost_context_destroy( boost_context* fc); boost_context_make is a merge of get_context with make_context + some fields assignations. "A user will have lower expectations from such a low level API and at the same time will give more freedom for other people to implement on top of it higher abstractions as Boost.Corutines, Boost.Fibers ..." - A non-copiable but movable safe context class on top of the low level C-like interface, that will provide modern and efficient C++ interface, that is usable by a user in a safe way. The safe context class would just be one of the client of the C-like interface, other library authors could either use the underlying C-like API directly or the class context depending on wether they need the speed or safety. * The context class will not be templated any more, and the use of type erasure will ensure that. * The constructor will accept any C++ callable, function pointers, functors, ... * At this high level interface, the function provided by the user could throw exceptions, and the library must wrap the user function and needs to store the exception raised and provide an interface to retrieve them. 2. The stack implementation must ensure that the stack is unwound before it is destroyed. A stack should only be safely deallocated if either has been completely unwound or if it doesn't own any resources. A basic stack without protection seems mandatory. For debug purposes it has been required to provide some stack usage functions that allow to know the max size used for example. Implementation 3. The assembler implementation on Windows must be disallowed if the ABI bug reported by Holger is confirmed. As far as Windows provides a fiber implementation that is as efficient as the assembler one, the assembler implementation is not mandatory and the user of the provided Windows fiber is a preferable choice. 4. Most of the 'throw' statements in the context class should actually be asserts. 5. The provided stack should be used to store any context parameters as far as no major impediment is found during the implementation. This will avoid further allocations on the safe context class. Documentation 6. The documentation is very minimalist and should be expanded with a discussion on context switching and use cases. Take care of all the grammar/spelling improvements suggested by the reviewers (It will be great if Jeffrey could review the documentation before the mini-review). Some user land examples should be provided (why not the yield example). 7. The concept of (Movable) Stack must be clearly defined, as it is done in the standard, using expression requirements with semantic constraints. 8. The assembler implementation should be carefully documented, so others could maintain the code and add other implementations if needed. 9. Some performance measure should be added. Providing the number of cycles on each implemented platform would be nice. Build system 10. The default build on a platform must work (It must be ABI compatible with the underlying platform) and take care of the specific features. Of course, the build needs to support cross compiling, so the user should be able to override the default values. Others points that are not required but that can make the library more usable or efficient: 1. The assembler implementations could be simplified if the context itself was represented as a single pointer to the top of the stack. This would save some instructions to move the source address from the stack return address to the context struct and back. 2. Instead of supporting a 'fc_link' a-la ucontext, have the callback function return a new context (it is void now). 'fc_link' is useful only on some limited cases (usually when you have an external scheduler managing your contexts), while the having the callback return the 'next continuation' has more applications (note that fc_link can be implemented easily in term of the continuation returning variant, but the reverse is harder). This will be very useful, for example, to implement some kind of 'exit_to' functionality (unwind the stack and restore another context), a key piece for safe context destruction. 3. Give to boost_context_swap the ability to exchange a parameter with the other context: boost_context_parm_t boost_context_swap( boost_context * ofc, boost_context const* nfc, boost_context_parm_t parm) where boost_context_parm_t is either union boost_context_parm_t { void * ptr; int int_; }; or simply intptr_t; boost_context_swap saves the current context in ocf and restores the context ncf as usual; additionally, if ofc was 'captured' on a call to boost_context_swap, this call will return the value of parm on the original context. This extra parameter is equivalent to the second parameter of longjmp. If ofc was created with boost_context_swap, 'parm' will be an extra parameter to the function. Note that this can be potentially implemented with little or no overhead. 4. Add a variant of boost_context_swap that allows executing a function on the destination context (in this case 'parm' would be passed parameter to this function. This is mostly useful for injecting an exception on the destination context. This can be potentially implemented on top of the existing functionality, but would slow down every context switch; a possible efficient implementation would manipulate the destination stack to add a call to the function on top of it, so that the cost is paid only when required. 5. Add a current context function (thread local). _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost