Re: [Boost-users] [Review.Coroutine] Coroutine review comments

Subject: Re: [Boost-users] [Review.Coroutine] Coroutine review comments
From: Nat Linden (nat_at_[hidden])
Date: 2012-09-10 13:12:08

On Sat, Sep 8, 2012 at 3:09 PM, Oliver Kowalke <oliver.kowalke_at_[hidden]> wrote:

> Am 07.09.2012 23:35, schrieb Nat Linden:

>> My intention was to test Oliver's Coroutine proposal by replacing it
>> into our code, but this is not possible without something equivalent
>> to 'future'.

> I decided not to provide the future<> facility becuase I've had some
> concerns about the 'correctness' of the interface.
>
> My intention is to provide a small, clean interface which is hardly to
> misuse.
> In my opinion a coroutine is a language-level construct allowing to enter a
> routine multiple times (by preserving the local state in the routine) ==
> multi-entry routine. Therefore I want a strong contract between the caller
> and the coroutine.
> This contract is established via the signature and the return type of the
> coroutine (coroutien-fn).
> Thus the interface provided by boost.coroutine must be stringent in the case
> that the coroutine can only activated/entered via coroutine<>::operator() -
> no other way to jump into the coroutines body (coroutine-fn) is possible.
> Additionally the return value and the coroutine parameter of
> coroutine<>::operator() must be the same as declared in the signature.
>
> The future<> concept in Giovannis library violates this contract. You have
> additional ways to jump into the coroutine's body. You can pass other
> parameters and return a different type than the coroutine signature defines.
> I think this violates the design.

This sounds like a classic collision between theory and practice. All
I can say is that we heavily rely on the functionality provided by
Giovanni's 'future' objects, and I cannot convert to a library that
does not provide something equivalent.

Our use case, as I said at C++ Now last May, seems fairly
straightforward. Giovanni's Coroutine library allows us to write code
that invokes asynchronous operations, yet retains the simplicity and
maintainability of using blocking operations. Cooperative context
switching is essential to us: we cannot use a new thread for this
purpose due to the prohibitive cost of discovering and protecting
accesses to all shared data objects.

I've been using the term 'coroutine' for this tactic because Giovanni
does. Perhaps that's an abuse of the theoretical concept of
"coroutine;" if so I apologize. But whatever you call it, we need that
API, or something very like it.

> I don't know your code but if you need 'future' semantics I suggest
> boost.fiber (http://ok73.ok.funpic.de/boost/libs/fiber/doc/html/,
> http://ok73.ok.funpic.de/boost.fiber.zip). boost.fiber provides lightweight
> threads using boost.context (context switching).

Thank you for the links. I will look it over.

I'm sorry to say that I've overlooked boost.fiber until now because
(a) I don't believe it was announced or discussed on the boost-users
mailing list, and I'm not on the boost developers' list; and (b) when
I see the word "fiber" in the context of program context, I
immediately think "Windows-specific." I will be glad to be wrong in
that assumption.

> The lib provides classes
> like mutex, condition-variables, event-variables and  futures. You can use
> it like boost.thread - but it provides cooperative multitasking.

Hmm! Dangerous though it is to shoot off my mouth before even starting
to read about boost.fiber, I immediately wonder about the need for
mutex in the context of cooperative context switching. Time to stop
speculating and start reading. ;-)

========================================================================
What follows are responses to specific documentation comments.

>> "The maximum number of arguments of coroutine-function is 10."
>
> it's a limitation of boost.tuple

Good. Mentioning that in the documentation's Note would permit an
interested party to consult Tuple documentation to discover how to
lift the limit if needed.

>> "An exception thrown inside coroutine-function (transfered via
>> exception-pointer - see Boost.Exception for details and requirements)
>> will be re-thrown by coroutine<>::operator()()."

> I gave the hint that the docu of boost.exception will describe all the
> requirements.

Thank you. You're correct that this permits me to research it myself.
Anything more is simply to inform a lazy reader. :-)

> Usally the used should not let an exceptio nescape from a coroutine-fn.

That's an interesting remark, and I would like to understand better
why you say that. I may well instantiate a coroutine with the
assumption that low-level exceptions in (code called by)
coroutine-function will be handled by the code that instantiates the
coroutine. Am I misguided?

> In the worst case you get an exception of type unknown_exception rethrown.

Good. It might be worth asserting in the documentation that -- even
though coroutine-function should ideally have a top-level try/catch
construct -- the library ensures that an exception in
coroutine-function will never be silently swallowed.

>> "The first argument of generator-function must be of type
>> generator<>::self_t, ..."
>>
>> The "only" argument?
>
> You could bind parameters to function entry

As I said in my previous note, it seems worth a few words in the
Generator documentation to distinguish between the generator-function
(possibly a bind() expression using any of several available bind()
implementations) and the C++ function bound by that bind() expression.
While the C++ function is of primary interest to the coder, and the
bind() expression seems a mere detail -- the coroutine library sees it
differently. Requirements on the coroutine-function and
generator-function apply to the actual expression (possibly a bind()
expression) passed to the coroutine or generator constructor. The
bound C++ function is effectively invisible to the coroutine library.

It is from that perspective that I would clarify that
generator-function must accept exactly one argument of type
generator<>::self_t.

> yield_break() was removed from the generator<>::self_t class (in git repo)

Thank you. I'm glad the coder doesn't have to decide whether to
execute 'return' or 'yield_break()'.

>> "generator-function is invoked the first time inside the constructor
>> of generator."
>>
>> That makes no sense to me. Is the value passed to the first yield()
>> call simply discarded? If so, why?
>
> I tried to express that a generator must be tested before you can use it:
>
> gen_t gen(...);
> if ( gen) {
>    int x = gen();
> }
>
> in order to know if gen is valid (== it will return a value) the
> return-value must be fetched from the generator-fn.

Oh! Light belatedly dawns. So a generator object constructed with a
generator-function with an empty body would immediately test 'false'.

That implies that the generator object buffers the value passed to
yield() until operator() is called to retrieve it. That seems worth
mentioning. It would have corrected my incomplete mental model.

>> "If generator-function can not return valid values anymore
>> generator<>::self_t::yield_break() should be called. This function
>> returns the execution control back to the caller and sets the
>> generator to be complete (is_complete() returns true)."
>>
>> is_complete() is not documented for boost::coro::generator.
>
> yes - you have to use generator<>::unspec_bool() or generator<>::operator!()

So the reference to is_complete() in the paragraph quoted above should
be removed or amended.