Re: [Boost-users] [signals] sandbox version status

From: Timmo Stange (ts_at_[hidden])
Date: 2007-02-11 17:58:52

Frank Mori Hess wrote:

> Ack!  You're pulling ahead of me :)  I've just got to the point where I'm 
> about to start going through the tests with thread_safe_signal.  A bit of 
> a redundant effort, I know, but once I got it to where I was planning to 
> drop it, it seemed so close to being done.  Better to have two thread-safe 
> implementations than zero, I suppose.

It's actually the best way to combine efforts in this case. There's
not enough independent functionality to work separately on one
implementation, I think, but we've each discovered some problems
that the other might have missed. I was not aware that the combiner()
member was public for example and would have perhaps missed that
loophole for a long time.

> I didn't really follow exactly why you decided to drop Group and 
> GroupCompare.  I was able to implement support for them pretty trivially 
> in thread_safe_signal (see thread_safe_signals/detail/slot_groups.hpp and 
> the connect() functions in 
> thread_safe_signals/detail/signal_template.hpp).

I'm not so happy with the preprocessor flag solution and I might
decide to dispatch to the right implementation based on the Group
template parameter alone at compile time, choosing the faster
approach for signals::no_name.

The thought behind all this is the infamous template code bloat
problem, which I carry around more like a religious belief, because
my compiler handles it rather well, but as so many bright people
have mentioned it, I take it as a given. For every template you
use, the compiler may create code for each instantiation potentially
in every compilation unit. It gets worse with polymorphism, because
you make it harder for the compiler to optimize code redundancy
away (as those functions are usually called through a fixed function
pointer). I don't know if you looked at the Boost.Function code.
They put a lot of effort into avoiding this problem (as opposed to
Loki's generalized functor, which I looked at first).

To make a long story short, Doug also worked hard on putting the
majority of the code in non-templated classes and I wanted to
retain that policy as much as possible (I'm usually the kind of
guy that is fast to say those people should upgrade their
compilers, so this is perhaps a good lesson for me ;)). The
named_slot_map in Boost.Signals is non-templated and the only
way I see for now to improve its performance is to make it
templated.

As for your solution: If I understand it correctly, your signal
does not support the full ordering of the original version. You
can establish an order (through at_back/at_front) within the groups
and for the ungrouped slots in one signal. The standard containers
don't support that notion. You'd need a (meta-)key with an infinite
value range to support at_back/at_front in a std::map.

> thread_safe_signals should be able to do multiple signal invocations 
> concurrently, as well as disconnect and connect concurrently with 
> invocation, due to my much-maligned per-slot locking :) and some other 
> trickery.  If you benchmark thread_safe_signals though, be sure to do it 
> with multiple slots connected and multiple threads on a multicore cpu so 
> its benchmark results look nice and inflated :)
> 
> However, you should be able to get concurrent signal invocation even with a 
> single mutex once boost supports a recursive_read_write_mutex, right?

Yes, that would be possible at the expense of copying the combiner for
the invocation, which is acceptable and parameterizable through the
ThreadingModel anyway.

Regards

Timmo Stange