[signals2][review] The review of the signals2 library is scheduled to end on Monday, November 10th

Hello, The review of the signals2 library is scheduled to end on Monday, November 10th. We have had some good discussion so far and a couple of reviews. I hope the remaining few days will provide enough time to others that were planning on submitting a review. If anyone needs any more discussion time or time to write the review, please let me know (so we can see about extending the review period, or so that I know to wait for your review longer). Kind regards, Stjepan

Hello,
The review of the signals2 library is scheduled to end on Monday, November 10th. We have had some good discussion so far and a couple of reviews. I hope the remaining few days will provide enough time to others that were planning on submitting a review.
If anyone needs any more discussion time or time to write the review, please let me know (so we can see about extending the review period, or so that I know to wait for your review longer).
Kind regards,
Stjepan _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hi all, my result of the review: * What is your evaluation of the design? Note: I've mainly focused on a user point of view for this review. At the end of the review I've taken some look into the libraries source code. Normally the first contact with a new library is the libraries documentation. At boost.signals2 library documentation the design is not really clear. Ok, the signal-slot mechanism should be quite clear for most of software developers but the documentation could be more detailed concerning to basic mechanisms. Boost.signals2 wants to be a thread safe library. In the documentation there is nothing about the design in relation to a thread context. * What is your evaluation of the implementation? Like I've said above, my review focused mainly the user point of view. At the end I've taken a quick look into the source code. The code is quite hard to read for my taste. It's not a well formatted source code. Sometimes signatures are not well structured and ordered. The signal class would be a good example here: template<typename Signature, typename Combiner = optional_last_value<typename boost::function_traits<Signature>::result_type>, typename Group = int, typename GroupCompare = std::less<Group>, typename SlotFunction = function<Signature>, typename ExtendedSlotFunction = typename detail::extended_signature<function_traits<Signature>::arity, Signature>::function_type, typename Mutex = mutex > class signal: public detail::signalN<function_traits<Signature>::arity, Signature, Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::type If the user wants to use a different mutex (e.g. 'dummy_mutex') it must take the default values for all the other template arguments. This could lead to something like this: typedef signals2::signal < void (bool), signals2::optional_last_value<typename boost::function_traits<void (bool)>::result_type>, int, std::less<int>, function<void (bool)>, typename signals2::detail::extended_signature<function_traits<void (bool)>::arity, void (bool)>::function_type, signals2::dummy_mutex
signal_t;
This isn't really user friendly. But it's sometimes a matter of taste. * What is your evaluation of the documentation? Ok, but not satisfying at all. I liked the splitting of the tutorials into the sections beginner, intermediate and advanced. A libraries newbie could easily navigate through the beginners sections and than towards to the more difficulty areas. This is a good practise to take the user through the documentation by hand. I don't liked the level of detail of the examples. Before I began with the review of boost.signals2 I read about the boost.signals library in 'Beyond the C++ Standard Library' from Bjorn Karlsson. Without the knowledge of this article in this book, the examples don't give the user the imagination what could be done with boost.signals2. More detailed and straight forwared examples would help the users very much. I also don't liked the fact that only three times the word 'thread' was named. Boost.signals2 wants to be a 'thread safe signals' library. Therefore some more details and especially more examples concerning on that area should be given to the user. During the review I've read about the 'signal::extended_slot_type'. But there was no example about that slot type. No deeper hint in the documentation only one sentence about that. The explanation of the author was that extended slots is a feature that comes to the library in the last minutes. * What is your evaluation of the potential usefulness of the library? In general, a thread-safe implementation of a signal slot mechanism could be an enrichment of an application for todays multiprocessor processors. * Did you try to use the library? With what compiler? Did you have any problems? I wrote some single-threaded and some multi-threaded test programs. The library was easy to use. No exceptions, no errors. The used compilers are the Microsoft Visual Studio 2008 (SP1) under Windows XP and GCC 4.0.2 under Mac OS X 10.5.5. The only problems I've had was under Windows. The signals2-2008-10-08 produces some compiler errors and warnings. For this reason I used signals2-2008-10-22 at Windows. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I've spend some time to read about boost.signals1 at 'Beyond the C++ Standard Library' from Bjorn Karlsson to get a feeling about the original version of the boost.signals library. I've also spend some days to write test programs and to study the source code of the library. However, I've tried to focus on a users point of view. * Are you knowledgeable about the problem domain? I know some signal slots implementations like the one from Trolltechs Qt. I also know threads and the usage of them. * Do you think the library should be accepted as a Boost library? NO, not now! But with some modifications in some weeks. I think that a thread-safe version of a signal-slot library is a must-have for the boost library. Regarding to the current version of boost.signals2 library (signals2-2008-10-08.zip) I have the feeling that the library is at alpha state. Like I've wrote before the documentation is not very detailed. To less examples to less design and detail informations about the thread safety. Undocumented functionalities like 'extended_slot' and some more. At the boost.devel newsgroup some performance outputs are posted. One benchmark was very interesing: "On Monday 03 November 2008 13:31, Michael Marcin wrote: > Frank Mori Hess wrote: > > boost::signal, 10 connections, tracking enabled, invoking 1000000 > > times: 0.78 s > > > > boost::signals2::signal, 10 connections, tracking enabled, invoking > > 1000000 times: 4.92 s > > Over 6 times slower in this case. That seems to warrant some concern. Almost 2 seconds of the overhead is due to atomic reference counting for shared_ptr. If I compile the benchmark with DISABLE_BOOST_THREADS I get: boost::signals2::signal, 10 connections, tracking enabled, invoking 1000000 times: 3.20 s [...]" The former version of boost.signals needs 0.78s for the test, the new version needs 3.20s. This supports my feeling that there could be done more optimication at boost.signals2. To cut a long story short boost.signals2 should be part of boost. Definitive! But I think some weeks of work could help the library very much. Franz

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Monday 10 November 2008 05:09 am, Franz Alt wrote:
If the user wants to use a different mutex (e.g. 'dummy_mutex') it must take the default values for all the other template arguments. This could lead to something like this:
typedef signals2::signal < void (bool), signals2::optional_last_value<typename boost::function_traits<void (bool)>::result_type>,
I'm just nitpicking here, but a user would just write: signals2::optional_last_value<void> for the Combiner,
int, std::less<int>, function<void (bool)>, typename signals2::detail::extended_signature<function_traits<void (bool)>::arity, void (bool)>::function_type, signals2::dummy_mutex
and a user would write function<void (const signals2::connection &, bool)> for the ExtendedSlotFunction.
signal_t;
This isn't really user friendly.
That's true. The Mutex parameter got put at the end because it was a new addition and putting it at the end was more backwards compatible. The relevant suggestions on dealing with this have been: removing some of the parameters like SlotFunction and ExtendedSlotFunction (my suggestion), and using Boost.Parameter (as Vincent Botet suggested). One problem with Boost.Parameter is how to document it in the Signals2 reference. See the "best practices: documentation" section of the Boost.Parameter docs: http://www.boost.org/doc/libs/1_37_0/libs/parameter/doc/html/index.html#docu... Another possibility might be to remove Mutex as a template parameter from the signal class and instead pass the mutex type to the signal constructor, so only the constructor would be templated on the Mutex type.
The former version of boost.signals needs 0.78s for the test, the new version needs 3.20s. This supports my feeling that there could be done more optimication at boost.signals2.
Yes, the tracking overhead speed can be optimized further. To be clear though, the boost::trackable connection management in Boost.Signals can have zero overhead on invocation, while the shared_ptr/weak_ptr will always have some overhead due to locking the tracked weak_ptr into shared_ptr before running the slots. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFJGFrc5vihyNWuA4URAqAeAJ0enqxTJm7ZzNkcPTbM3ia8mytRPgCgq406 SslpCWPZJ9ZzvAlDZkt86i0= =M4Wy -----END PGP SIGNATURE-----

on Mon Nov 10 2008, Frank Mori Hess <frank.hess-AT-nist.gov> wrote:
One problem with Boost.Parameter is how to document it in the Signals2 reference. See the "best practices: documentation" section of the Boost.Parameter docs:
http://www.boost.org/doc/libs/1_37_0/libs/parameter/doc/html/index.html#docu...
That problem is not so hard to solve that it should discourage anyone from using the library. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
participants (4)
-
David Abrahams
-
Frank Mori Hess
-
Franz Alt
-
Stjepan Rajko