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