Re: [boost] Synapse library review starts today December 2

Subject: Re: [boost] Synapse library review starts today December 2
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2016-12-04 17:20:12

On Sun, Dec 4, 2016 at 4:28 AM, Klemens Morgenstern <
klemens.morgenstern_at_[hidden]> wrote:

> Am 04.12.2016 um 11:44 schrieb Emil Dotchevski:
>
>> On Sat, Dec 3, 2016 at 7:06 AM, Klemens Morgenstern <
>> klemens.morgenstern_at_[hidden]> wrote:
>>
>> For your review you may wish to consider the following questions:
>>>
>>>>       - What is your evaluation of the design?
>>>>
>>>> The way I see it, it tries to solve a specifiy style problem for signals
>>> in a very simple and straight-forward way.
>>>
>>> But: if it uses C++11 (like with type_traits) why not go all the way and
>>> std::shared_ptr instead of boost::shared_ptr?
>>>
>>
>> In synapse/dep/smart_ptr.hpp you can change which shared_ptr/weak_ptr you
>> want to use, though this should probably be controlled by a configuration
>> macro. However, if this is a Boost library, it should use
>> boost::shared_ptr.
>>
> Not as I understand this sentence:  "It also means using the C++ Standard
> Library where applicable."
> http://www.boost.org/development/requirements.html
>

If the rules say that Boost libraries shouldn't use boost::shared_ptr but
std::shared_ptr if available, I'll comply (though I can't imagine why this
would be a good idea.)

> The dependence on C++11 is only for thread_local and for correct concurrent
>> initialization of static objects. The library is still very useful without
>> these, it's just that the (optional) interthread communication support
>> won't work.
>>
> If you have the library for the most part in C++98 and some extra features
> in C++11 that does make sense. But I think if you want to do that, go for
> one standard, and thus either use everything from C++11 or
> http://www.boost.org/doc/libs/1_62_0/doc/html/thread/thread_
> local_storage.html
>

I considered using boost::thread_specific_ptr but to my understanding its
initialization must be complete before it is used by different threads.
This makes it impossible to use in Synapse because it uses thread local
storage per type inside function templates, which would require C++11
concurrent initialization of static objects (please do correct me if I'm
wrong). So, I opted for a cleaner implementation using C++11 only for these
two features (thread_local and concurrent initialization of static objects).

> Why are there no move-operations for example for connections. It doesn't
>>> make sense to put this into a shared_ptr, when you can make it movable,
>>> while disabling copies.
>>>
>>> Using shared_ptr to hold on to connections is a design choice.
>>
> I've gathered that much, I'm just questioning it ;). It makes sense if you
> are pur C++98, but not if you have C++11. And I do think this is a bad
> choice for C++11.
>

Strictly speaking, using shared_ptr does use move since the shared_ptr
itself is moveable. :)

I guess we'll agree to disagree, in my opinion refcounting is more
appropriate for connections compared to move semantics.

> Also: why can't return values be combined as in boost.signals2? That
>>> should at least be an optional feature.
>>>
>>> See http://zajo.github.io/boost-synapse/questions_and_answers.html.
>>
> I know, but that doesn't answer the question, really. Because the
> reasoning can also be applied to boost.signals2 also, because signals in
> general don't care how many functions are connected.
>

What I mean is that I can be persuaded that the benefits of this feature in
signals2 somewhat outweight the added complexity, but in Synapse the return
type is used to tell apart different signals with otherwise the same
signature, which is quite practical in my experience. Perhaps I'm missing
some important use cases but I've never missed that particular feature.

> Now the slot-lifetime is determined by weak_ptr, if I understand
>>> correctly. I don't think that's generic enough at all, there should be
>>> more
>>> solutions. One would be, that target can inhert some sort of slot-type,
>>> which will handle that. That way I could use the signals on elements on
>>> the
>>> stack, and also do this unintrusively:
>>>
>>> struct signalable_vector : std::vector<int>, slot_type {};
>>>
>>> Some solution for unique_ptr is needed. A solution would be to implement
>>> a
>>> synapse::unique_slot_ptr which implements that.
>>>
>>> That's what the null_deleter in shared_ptr is for.
>>
> How does that help with unique_ptr?
>

Is there something like weak_ptr that works with unique_ptr? The point of
passing weak_ptr to connect is that emit won't call the connected function
after the emitter (or the target) have expired, even if the connection
object is still afloat. When it is impossible to use weak_ptr for this,
then the only option to stay safe is to control the lifetime of the
connection object (and pass a raw pointer to connect<>) .

> If I understand it correctly, interthread signaling will be used if emit
>>> is called on a different thread than connect was? I think that's probably
>>> an alright default, but bad design if it's always the case. Either
>>> provide
>>> a thread-affinity for each object, which can be changed, or add a
>>> dispatcher to make it explicit.
>>>
>>> Do you mean provide thread affinity for the connection objects? That can
>> be
>> done.
>>
> Yes, that is what I meant. Alright, seems like I missed that.
>

Oh, you misunderstand. I meant that I can see that this may be needed and
that it can be implemented, not that it is implemented already.

> I would personally prefer a dispatcher, which would not only allow to
>>> dispatch it to a std::thread through thread_local_queue but also to
>>> something like a boost::asio::io_service. The io_service can actually be
>>> executed from several threads, so you don't know which one will actually
>>> poll it.
>>>
>>> Do you mean that you want threads other than the connecting thread to be
>> able to poll for queued signals? Why not just connect from these other
>> threads?
>>
> Well because I might have 3 thread polling work from the io_service - how
> would I know on which one the signal ends up?
>

If multiple threads create thread_local_queue objects, emit<S> will queue
the S signal in all threads in which S was connected. I've taken great care
to avoid overhead in emit<S> in case threads that have created
thread_local_queues have not connected S.

      - What is your evaluation of the potential usefulness of the
>>>
>>>>         library?
>>>>
>>>> I don't think it's that useful, but I guess I would still use
>>> boost.signals2 and boost::asio::io_service for interthread signaling. I'm
>>> not convinced this should be a distinct library, and not a feature of
>>> boost.signals2.
>>>
>>> Synapse does not claim to be a "better" Signals2; its usefulness is not
>> as
>> an alternative to Signals2 but as a solution to problems Signals2 can not
>> solve. Here are three examples of this, though I have many more:
>>
>> http://zajo.github.io/boost-synapse/handling_events_from_an_
>> OS_message_pump.html
>> http://zajo.github.io/boost-synapse/adding_custom_signals_to
>> _Qt_objects_without_MOCing.html
>> http://zajo.github.io/boost-synapse/using_meta_signals_to_co
>> nnect_to_a_C-style_callback_API.html
>>
>> How about:
>
> template<typename Signature>
> inline boost::signals2::signal<Signature> & impl()
> {
>    boost::signals2::signal<Signature> sig;
>    return sig;
> }
>
> template<typename Signature, typename Func>
> boost::signals2::connection connect(Func f) {return
> impl<Signature>().connect(f);}
>
> template<typename Signature, typename ...Args>
> void emit(Args... args) {return impl<Signature>()(args);}
>
> Let's say I build a more elaborate version of this - how does that compare
> to synapse?

I suppose if it is elaborate enough you'd rediscover Synapse, but with a
different low level machinery, though I suspect that both libraries contain
incompatible intricacies which make sense in the different approaches they
take, since non-intrusiveness (in the way Synapse is non-intrusive) was not
a design goal of Signals2.

Emil