[Review.Coroutine] Vicente's review
Hi, I understand that Oliver is busy as all of us and that it is better to provide first the minimal and well designed scope he is confident with. I expect however that Oliver will add soon symmetric coroutines based on the work done by Giovanni in a near future. - What is your evaluation of the design? I have some suggestions about changing the interface, but I'm not sure all of them could be implemented in an efficient way. * The use of thecoro_t::self_t & self parameter makes too much noise. This parameter must be forwarded to any function that could yield the coroutine. An alternative design using a thread specific pointer could allow to access the current coroutine int f20() { typedef this_coroutine<int(void)> this_coro; this_coro::yield(5); } Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided. * There is an asymmetry between the value returned the last time and the others. self.yield( i); // OTHERS return i; // LAST This asymmetry forces the user to manage the last return in a specific case, changing the readability of the code. I would prefer coroutine function returns void. voidfn( coro_t::self_t & self) { self.yield(1); self.yield(2); } * If this is not adopted, the implementation should check at compile time that the return type of the coroutine function is the one of the signature. * The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed. Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters. int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); } int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); } Or even use some kind of bind parameter int f20(coro_t::self_t & self) { int i; int j; self::bind(i, j); self.yield( i + j ); } int f20() { int i; int j; typedef this_coroutine<int(int)> this_coro; this_coro::bind(i, j); this_coro::yield( i + j ); } If bind is adopted, the implementation could use it to store directly the actual parameters, instead of needing to store them in the self_t instance and them returning them when yield is called. */As// Boost.Coroutine is able to transfer user exceptions, and there is no difference between calling to coroutine<>::self_t::yield_break()/ and throwing/coroutine_terminated,/both (/coroutine<>::self_t::yield_break()/ and throwing/coroutine_terminated)/be removed from the library. The same applies for generators. * noexcept specifier should be added to class coroutine { // ... coroutine() BOOST_NOEXCEPT; coroutine( coroutine && other) BOOST_NOEXCEPT; coroutine & operator=( coroutine && other) BOOST_NOEXCEPT; explicitoperator bool() const BOOST_NOEXCEPT; bool is_complete() const BOOST_NOEXCEPT; //... }; void swap( coroutine & l, coroutine & r) BOOST_NOEXCEPT; The same applies to generator. * Type-erase on the Allocator parameter as the standard std::packaged_task does. * Make coroutine/generator classes to follow the allocator aware standard protocol adding the constructor with the standard allocator. A specializations of the use_allocator template should be added for this purpose. * Moving some of the coroutine/generator optional parameters to a coro::attributes class could simplify the interface std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, * The lib shouldn't throw when preconditions are not satisfied, assert should instead. * It is a shame that the library can not support completely move semantics for the coroutine parameters, as the used boost::tuple don't support it yet. I would like this point be addressed before the first release so that there is no break in the interface when this will be solved either using boost::tuple, boost::fusion::tuple or std::tuple. At a last resort the library could either define a macro that allows to parameterize the tuple implementation, or alternatively it could implement a wrapper that forwards to the best implementation. * As others have noted, providing a view of a generator as an output iterator will be welcome. Viewing it as a range will be also appreciated. BTW, I don't like the alternative using optional as the result of a generator. Following the output iterator concept the access is defined only if the generator has not finished. - What is your evaluation of the implementation? I have not a deep opinion as I have not found the time to inspect it. At first glance, it seems that the inheritance hierarchy is deeper than needed and the use of the pre-processor to manage with variadic templates hides a possible good design (as it is usual in those cases). Maybe now that most of the c++ compilers support them, using the preprocessor is less imperative. - What is your evaluation of the documentation? I find the documentation a little bit sparse and contain a lot or typos and minor errors. I would appreciate if the original Boost.Coroutine documentation from Giovannii were not lost, at least for Motivation and Tutorial parts. I would appreciate also if the documentation include the rationale to don't include symmetric coroutines and show how the current library can be used to solve the problems the missing features provide, why not as examples. * showing the signature in the documentation will be more informative. template< typename Signature, typename Allocator = ctx::stack_allocator
class coroutine; template< typename R, typename ...ArgTypes, typename Allocator
class coroutine<R(ArgTypes...)> { public: class self_t { public: tuple<ArgTypes...>yield( R); void yield_break(); }; * documentation for the following constructor is missing in the reference template< typename Fn > coroutine( Fn && fn, std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, Allocator alloc = Allocator() ); *The example typedef boost::coro::coroutine< void(int) > coro_t; void f( coro_t::self_t & self, int i) { ... self.yield(); ... } coro_t c( boost::bind( f, _1) ); c( 7); doesn't compiles. It should be something like coro_t c( boost::bind( f, _1, _2) ); BTW, I don't see the need to use boost::bind in this case, as coro_t c( f ); works as well. The superfluous uses of bind in the documentation should be removed: - What is your evaluation of the potential usefulness of the library? Very useful. coroutines and generators are some useful constructions that are missing in the language. - Did you try to use the library? With what compiler? Did you have any problems? Yes. I used gcc-4.2.1,4.6.0, 4.7.0,clang2.9 andclang-3.0 on darwin. I had no problem running the provided examples and tests. I have written some examples. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I have read the documentations of Oliver and Giovanii libraries, followed the review and used the library. - Are you knowledgeable about the problem domain? From the theoretical point of view, yes. Never used in practice. Finally, my vote is YES. Even if the current interface has some drawbacks that should be mentioned in the documentation, I think I Boost.Coroutine is already useful and should be accepted into Boost. Best, Vicente P.S. Maybe I have forgotten to include some of the remarks I did on preceding posts.
* The use of thecoro_t::self_t & self parameter makes too much noise. This parameter must be forwarded to any function that could yield the coroutine.
An alternative design using a thread specific pointer could allow to access the current coroutine
int f20() { typedef this_coroutine<int(void)> this_coro; this_coro::yield(5); }
I was also thinking about this issue (as I already implemented it in boost.fiber) but how do I know in f20() which coroutine is active (f20() is bound to)? In the case of boost.fiber I get this info from the internal scheduler but boost.coroutine hasn't one (and shouldn't).
Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided.
At least it would introduce additional indirections.
* There is an asymmetry between the value returned the last time and the others.
self.yield( i); // OTHERS return i; // LAST
This asymmetry forces the user to manage the last return in a specific case, changing the readability of the code. I would prefer coroutine function returns void. voidfn( coro_t::self_t & self) { self.yield(1); self.yield(2); }
generator<> now requires a return type of 'void' of the generator function. This makes sense because generator< R >::operator() returns a optional< R > (Eugene's suggestion). generator< int > gen( f); if ( optional< int > val = gen() ) {...} if the generator function simple returns (no yield() called) then gen() returns a none (== invalid/unset optional). In the case of coroutines coroutine<R>::operator() should still return R (not optional<R>). void f( self_t &) {} coroutine< string( int, int) > coro( f); string s = coro( 1, 2); Which value should coroutine< string >::operator() return if the coroutine-function is required to return void and the user simply returns without calling yield()? My impression is that this asymmetry between coroutines and generators should be kept. * If this is not adopted, the implementation should check at compile time
that the return type of the coroutine function is the one of the signature.
done for coroutine and generators (prevent also void as return type for generators).
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); }
interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>().
* As others have noted, providing a view of a generator as an output iterator will be welcome. Viewing it as a range will be also appreciated. BTW, I don't like the alternative using optional as the result of a generator. Following the output iterator concept the access is defined only if the generator has not finished.
hmm - I've already adopted returning optional. couldn't the iterator build on top of the current interface? generator< X > gen( f); generator_iterator< X > i( move( gen) ); generator_iterator< X > e(); for (; i!= e; ++i) { * i; }
- What is your evaluation of the implementation?
I have not a deep opinion as I have not found the time to inspect it. At first glance, it seems that the inheritance hierarchy is deeper than needed
the code uses static polymorphism (curiously recurring template pattern), that means that you don't pay overhead. I was forced to split up the code into separate templates because of too much template specialization == specialize return type (separate void), specialize amount of function parameters -> defines coroutine<>::operator()( Arg0 ,...).
P.S. Maybe I have forgotten to include some of the remarks I did on preceding posts.
I've most of your suggestions already implemented. regards, Oliver
Oliver Kowalke wrote
* The use of thecoro_t::self_t & self parameter makes too much noise. This parameter must be forwarded to any function that could yield the coroutine.
An alternative design using a thread specific pointer could allow to access the current coroutine
int f20() { typedef this_coroutine<int(void)> this_coro; this_coro::yield(5); }
I was also thinking about this issue (as I already implemented it in boost.fiber) but how do I know in f20() which coroutine is active (f20() is bound to)? In the case of boost.fiber I get this info from the internal scheduler but boost.coroutine hasn't one (and shouldn't).
A coroutine runs on the same thread as its caller. The call can reset the self_t instance on a thread specific pointer so that it can be retrieve later on.
Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided.
At least it would introduce additional indirections.
I agree that a thread specific pointer is not gratuitous, but adding the self_t parameter over the whole stack has its cost also. Providing both approaches would let the user choose which one is best adapted to her context.
* There is an asymmetry between the value returned the last time and the others.
self.yield( i); // OTHERS return i; // LAST
This asymmetry forces the user to manage the last return in a specific case, changing the readability of the code. I would prefer coroutine function returns void. voidfn( coro_t::self_t & self) { self.yield(1); self.yield(2); }
generator<> now requires a return type of 'void' of the generator function. This makes sense because generator< R >::operator() returns a optional< R
(Eugene's suggestion).
generator< int > gen( f); if ( optional< int > val = gen() ) {...}
if the generator function simple returns (no yield() called) then gen() returns a none (== invalid/unset optional).
I don't think optional should be used. The user must know if it can call the generator to get the next value or not. My point was independent of the generator interface. I don't think the asymmetry is beneficial, but maybe I'm missing something important.
In the case of coroutines coroutine<R>::operator() should still return R (not optional<R>).
void f( self_t &) {} coroutine< string( int, int) > coro( f); string s = coro( 1, 2);
Which value should coroutine< string >::operator() return if the coroutine-function is required to return void and the user simply returns without calling yield()?
A coroutine should yield a value once it is called/resumed. The library can check that a value has been yield when the function returns, isn't it?
My impression is that this asymmetry between coroutines and generators should be kept.
I would like to be convinced. For the time been I'm not.
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); }
interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>().
* As others have noted, providing a view of a generator as an output iterator will be welcome. Viewing it as a range will be also appreciated. BTW, I don't like the alternative using optional as the result of a generator. Following the output iterator concept the access is defined only if the generator has not finished.
hmm - I've already adopted returning optional. couldn't the iterator build on top of the current interface?
generator< X > gen( f); generator_iterator< X > i( move( gen) ); generator_iterator< X > e();
for (; i!= e; ++i) { * i; }
The iterator interface don't need the use of optional. It need just to know if the generator has completed or not and only when not completed been able to retrieve the value.
- What is your evaluation of the implementation?
I have not a deep opinion as I have not found the time to inspect it. At first glance, it seems that the inheritance hierarchy is deeper than needed
the code uses static polymorphism (curiously recurring template pattern), that means that you don't pay overhead. I was forced to split up the code into separate templates because of too much template specialization == specialize return type (separate void), specialize amount of function parameters -> defines coroutine<>::operator()( Arg0 ,...).
I can understand your argument.
P.S. Maybe I have forgotten to include some of the remarks I did on preceding posts.
I've most of your suggestions already implemented.
Great. Best. Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/Review-Coroutine-Vicente-s-review-tp46356... Sent from the Boost - Dev mailing list archive at Nabble.com.
Am 13.09.2012 16:42, schrieb Vicente Botet:
generator< int > gen( f); if ( optional< int > val = gen() ) {...}
if the generator function simple returns (no yield() called) then gen() returns a none (== invalid/unset optional).
I don't think optional should be used. The user must know if it can call the generator to get the next value or not.
That was my previous design. The user has to check the generator like: if ( gen) before asking the next value if ( gen) { int x = gen(); ... } This design required pre-fetching and storing the return value via a context jump into the generator routine. Only with this mechanism I was able to know if the next call to generator<>::operator() will return a value. (requires that the first pre-fetch is done in the ctor of generator<> The difference to the actual design if ( optional< int > val = gen() ) { .... } is that I don't need to pre-fetch and not to store the optional<> as parameter (optional is only created inside generator<>::operator()).
My point was independent of the generator interface. I don't think the asymmetry is beneficial, but maybe I'm missing something important.
In the case of coroutines coroutine<R>::operator() should still return R (not optional<R>).
void f( self_t &) {} coroutine< string( int, int) > coro( f); string s = coro( 1, 2);
Which value should coroutine< string >::operator() return if the coroutine-function is required to return void and the user simply returns without calling yield()?
A coroutine should yield a value once it is called/resumed. The library can check that a value has been yield when the function returns, isn't it?
If the coroutine function does not call self_t::yield() at all no return value can be fetched and returned to the caller. That means in the corrected example above: void f( self_t &, int x, int y) {} coroutine < string( int, int) > coro( f); string s = coro( 1, 2); // f did not return a value, what value should s have after this? 's' would contain garbage - therefore I think the user must be forced to return an value at least at the final 'return' statement.
My impression is that this asymmetry between coroutines and generators should be kept.
I would like to be convinced. For the time been I'm not. I hope with my argument above you are convinced now ;)
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); } interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>().
Would not prevent user from doing something like this: in f20( coro_t::self_t & s, int x, int y) { s.yield( s.get< 0 > * x + y); } coroutine< int(int) > coro( bind( f20, _1, _2, 2, 3) ); int x = coro( 5); // x == 13 ? don't know which interface would be small and hardly to misuse ? regards, Oliver
This design required pre-fetching and storing the return value via a context jump into the generator routine. Only with this mechanism I was able to know if the next call to generator<>::operator() will return a value. (requires that the first pre-fetch is done in the ctor of generator<>
The difference to the actual design
if ( optional< int > val = gen() ) { .... }
is that I don't need to pre-fetch and not to store the optional<> as parameter (optional is only created inside generator<>::operator()).
If the generator-function returns non-void (original design) and has no yield_break, then it's possible for generator::operator() to return R (not optional<R>) and do no prefetching. But personally, I would rather take the optional<R> with a void generator-function and provide the iterator (iterator::operator* will return R, not optional<R>) as the preferred interface.
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); }
interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>().
One could still do this: int f20(coro_t::self_t & self) { X& x1 = self.get<0>(); self.yield(); X& x2 = self.get<0>(); // use x1 and x2 here. x1 might be bogus now } Maybe the example is a bit contrived but not anymore than the original one where the use of formal parameters posed the same danger. Even with bind(), one could create a ref/ptr to the bound variable that would be bogus after the next yield(). Regards, Eugene
Le 13/09/12 18:39, Eugene Yakubovich a écrit :
This design required pre-fetching and storing the return value via a context jump into the generator routine. Only with this mechanism I was able to know if the next call to generator<>::operator() will return a value. (requires that the first pre-fetch is done in the ctor of generator<>
The difference to the actual design
if ( optional< int > val = gen() ) { .... }
is that I don't need to pre-fetch and not to store the optional<> as parameter (optional is only created inside generator<>::operator()).
If the generator-function returns non-void (original design) and has no yield_break, then it's possible for generator::operator() to return R (not optional<R>) and do no prefetching. But personally, I would rather take the optional<R> with a void generator-function and provide the iterator (iterator::operator* will return R, not optional<R>) as the preferred interface. +1.
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); } interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>(). One could still do this:
int f20(coro_t::self_t & self) { X& x1 = self.get<0>(); self.yield(); X& x2 = self.get<0>(); // use x1 and x2 here. x1 might be bogus now } You are right. This is why I prefer the bind approach, if doable. Maybe the example is a bit contrived but not anymore than the original one where the use of formal parameters posed the same danger. Even with bind(), one could create a ref/ptr to the bound variable that would be bogus after the next yield(). I don't see the problem with bind. Could you show an example?
Best, Vicente
Maybe the example is a bit contrived but not anymore than the original one where the use of formal parameters posed the same danger. Even with bind(), one could create a ref/ptr to the bound variable that would be bogus after the next yield().
I don't see the problem with bind. Could you show an example?
My bad. I just realized that there is no problem with bind. I apologize for the false alarm. Regards, Eugene
Le 13/09/12 17:38, Oliver Kowalke a écrit :
Am 13.09.2012 16:42, schrieb Vicente Botet:
generator< int > gen( f); if ( optional< int > val = gen() ) {...}
if the generator function simple returns (no yield() called) then gen() returns a none (== invalid/unset optional).
I don't think optional should be used. The user must know if it can call the generator to get the next value or not.
That was my previous design. The user has to check the generator like:
if ( gen) before asking the next value
if ( gen) { int x = gen(); ... }
This design required pre-fetching and storing the return value via a context jump into the generator routine. Only with this mechanism I was able to know if the next call to generator<>::operator() will return a value. (requires that the first pre-fetch is done in the ctor of generator<>
The difference to the actual design
if ( optional< int > val = gen() ) { .... }
is that I don't need to pre-fetch and not to store the optional<> as parameter (optional is only created inside generator<>::operator()).
Why did you stored the pre fetched value in an optional<>l in the old design? Storing it in R seems enough to me, and with move semantics, moving it.
My point was independent of the generator interface. I don't think the asymmetry is beneficial, but maybe I'm missing something important.
In the case of coroutines coroutine<R>::operator() should still return R (not optional<R>).
void f( self_t &) {} coroutine< string( int, int) > coro( f); string s = coro( 1, 2);
Which value should coroutine< string >::operator() return if the coroutine-function is required to return void and the user simply returns without calling yield()?
A coroutine should yield a value once it is called/resumed. The library can check that a value has been yield when the function returns, isn't it?
If the coroutine function does not call self_t::yield() at all no return value can be fetched and returned to the caller. That means in the corrected example above:
void f( self_t &, int x, int y) {} coroutine < string( int, int) > coro( f); string s = coro( 1, 2); // f did not return a value, what value should s have after this?
's' would contain garbage - therefore I think the user must be forced to return an value at least at the final 'return' statement.
My impression is that this asymmetry between coroutines and generators should be kept.
I would like to be convinced. For the time been I'm not. I hope with my argument above you are convinced now ;) Yes, I am. Ensuring that the coroutine returns one argument before completing is a good think.
* The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed.
Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters.
int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); }
int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); } interesting idea, I find it better than the version using bind because you don't can't forget to 'bind'. you are always required to use self_t::get<>().
Would not prevent user from doing something like this:
in f20( coro_t::self_t & s, int x, int y) { s.yield( s.get< 0 > * x + y); }
coroutine< int(int) > coro( bind( f20, _1, _2, 2, 3) );
This should not compile, as the int parameter of the coroutine signature shouldn't appear any more on the coroutine function. You should do coroutine< int(int) > coro( bind( f20, _1, 2, 3) );
int x = coro( 5); // x == 13
? don't know which interface would be small and hardly to misuse ?
The user can use bind to get a function that has the signature int(coro_t::self_t &). Best, Vcente
Am 13.09.2012 18:43, schrieb Vicente J. Botet Escriba:
> Le 13/09/12 17:38, Oliver Kowalke a écrit :
>> Am 13.09.2012 16:42, schrieb Vicente Botet:
>>>
>>>>
>>>> generator< int > gen( f);
>>>> if ( optional< int > val = gen() ) {...}
>>>>
>>>> if the generator function simple returns (no yield() called) then
>>>> gen()
>>>> returns a none (== invalid/unset optional).
>>>>
>>> I don't think optional should be used. The user must know if it can
>>> call the
>>> generator to get the next value or not.
>> That was my previous design. The user has to check the generator like:
>>
>> if ( gen) before asking the next value
>>
>> if ( gen) {
>> int x = gen();
>> ...
>> }
>>
>> This design required pre-fetching and storing the return value via a
>> context jump into the generator routine.
>> Only with this mechanism I was able to know if the next call to
>> generator<>::operator() will return a value.
>> (requires that the first pre-fetch is done in the ctor of generator<>
>>
>> The difference to the actual design
>>
>> if ( optional< int > val = gen() ) {
>> ....
>> }
>>
>> is that I don't need to pre-fetch and not to store the optional<> as
>> parameter (optional is only created inside generator<>::operator()).
>>
> Why did you stored the pre fetched value in an optional<>l in the old
> design? Storing it in R seems enough to me, and with move semantics,
> moving it.
- return type of 'R &' would require that the member has to be
initialized in the ctor (before pre-fetch() )
- optional<> allows to express 'non set' / 'invalid' easily (for
instance in case of an exception)
Le 13/09/12 19:36, Oliver Kowalke a écrit :
> Am 13.09.2012 18:43, schrieb Vicente J. Botet Escriba:
>> Le 13/09/12 17:38, Oliver Kowalke a écrit :
>>> Am 13.09.2012 16:42, schrieb Vicente Botet:
>>>>
>>>>>
>>>>> generator< int > gen( f);
>>>>> if ( optional< int > val = gen() ) {...}
>>>>>
>>>>> if the generator function simple returns (no yield() called) then
>>>>> gen()
>>>>> returns a none (== invalid/unset optional).
>>>>>
>>>> I don't think optional should be used. The user must know if it can
>>>> call the
>>>> generator to get the next value or not.
>>> That was my previous design. The user has to check the generator like:
>>>
>>> if ( gen) before asking the next value
>>>
>>> if ( gen) {
>>> int x = gen();
>>> ...
>>> }
>>>
>>> This design required pre-fetching and storing the return value via a
>>> context jump into the generator routine.
>>> Only with this mechanism I was able to know if the next call to
>>> generator<>::operator() will return a value.
>>> (requires that the first pre-fetch is done in the ctor of generator<>
>>>
>>> The difference to the actual design
>>>
>>> if ( optional< int > val = gen() ) {
>>> ....
>>> }
>>>
>>> is that I don't need to pre-fetch and not to store the optional<> as
>>> parameter (optional is only created inside generator<>::operator()).
>>>
>> Why did you stored the pre fetched value in an optional<>l in the old
>> design? Storing it in R seems enough to me, and with move semantics,
>> moving it.
> - return type of 'R &' would require that the member has to be
> initialized in the ctor (before pre-fetch() )
> - optional<> allows to express 'non set' / 'invalid' easily (for
> instance in case of an exception)
>
>
The storage type of R& should be R*. This is how boost futures manage it.
Best,
Vicente
Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided.
At least it would introduce additional indirections.
I agree that a thread specific pointer is not gratuitous, but adding the self_t parameter over the whole stack has its cost also.
Providing both approaches would let the user choose which one is best adapted to her context.
I like not having the self parameter but using TLS can be slow. The use of Boost's thread_specific_ptr (which uses pthread api) is especially slow. Using gcc's __thread attribute is much faster (I think that's acceptable performance wise) but this route will make it harder to support all the compilers. Regards, Eugene
Le 13/09/12 18:44, Eugene Yakubovich a écrit :
Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided. At least it would introduce additional indirections.
I agree that a thread specific pointer is not gratuitous, but adding the self_t parameter over the whole stack has its cost also.
Providing both approaches would let the user choose which one is best adapted to her context. I like not having the self parameter but using TLS can be slow. The use of Boost's thread_specific_ptr (which uses pthread api) is especially slow. Using gcc's __thread attribute is much faster (I think that's acceptable performance wise) but this route will make it harder to support all the compilers.
I agree that having a portable and efficient implementation will not currently possible, but I expect that more a more compilers will finish by implementing and thread local variables. I'm sure it is worth implementing it and check the performances. Maybe it is possible to implement it on top of the current interface. I'm aware of the slowness of boost::thread_specific_ptr, and I'm analyzing the implementation in order to see how it could be improved. Best, Vicente
participants (4)
-
Eugene Yakubovich -
Oliver Kowalke -
Vicente Botet -
Vicente J. Botet Escriba