[boost] [review] Boost.Context mini-review starts today, January 2nd

Subject: [boost] [review] Boost.Context mini-review starts today, January 2nd
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2012-01-01 21:45:16

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).