[signals] thread-safe post-disconnect() cleanup
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 We could implement a thread-safe post-disconnect callback by using the existing tracking functionality. That is, the connection body could hold a "cleanup" shared_ptr with a custom deleter. The custom deleter would run the user's cleanup callback. A weak_ptr copy of the cleanup shared_ptr would be kept just like it was any other tracked object. When disconnect() is called, the cleanup shared_ptr would be reset. If the signal is being invoked concurrently when disconnect() is called, the cleanup code won't get run until the last invocation is done with the slot and destroys its copy of the cleanup shared_ptr. BTW, I've added support for a two-parameter version of signals::track() to thread_safe_signals/track.hpp. It seems to work okay, the only thing I don't like about it is it implies there a coupling required between the value and the tracked shared_ptr, when really there isn't. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3iTv5vihyNWuA4URAkzWAKDHL0MjcFGmAvKbWoBYmhNN5RiCMACgypCS SLJPrYcUaR0u4YTTS35lEZQ= =dCrK -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
We could implement a thread-safe post-disconnect callback by using the existing tracking functionality. That is, the connection body could hold a "cleanup" shared_ptr with a custom deleter. The custom deleter would run the user's cleanup callback. A weak_ptr copy of the cleanup shared_ptr would be kept just like it was any other tracked object. When disconnect() is called, the cleanup shared_ptr would be reset. If the signal is being invoked concurrently when disconnect() is called, the cleanup code won't get run until the last invocation is done with the slot and destroys its copy of the cleanup shared_ptr.
I think we should perhaps first decide on the interface and behavior before we discuss an implementation. That cleanup callback should be optional, without parameters and return value, right? I also think it should be called synchronously directly from disconnect if the slot is not running - otherwise from the executing thread directly after the slot returns. I don't directly see the necessity for a shared_ptr there and I have to express my performance worries again. As nice as shared_ptr's thread-safety is, it doesn't come for free. If you store the callback in a Boost.Function object maintained by a shared pointer, we're talking about a minimum of three heap allocations (2 for the function and 1 for the reference count) - with most allocator implementations, those allocations imply process-global synchro- nization. The smart pointer itself (including all temporary copies and the creation from a weak_ptr) uses atomic reference counting. Both are scalability issues (mostly the synchronization) and exceptionally heavy in simple absolute runtime cost (as much as 50 times the cost of a simple object copy and more, depending on the CPU and system). I really hate to play the optimization freak here, but we should keep an eye on those things, because if the abstraction the library provides comes at a too high runtime cost, the usability is severely limited. I usually avoid heap allocations in library code and if I need to have them, I try to provide an optional allocator template.
BTW, I've added support for a two-parameter version of signals::track() to thread_safe_signals/track.hpp. It seems to work okay, the only thing I don't like about it is it implies there a coupling required between the value and the tracked shared_ptr, when really there isn't.
Could you give a short example of how that would look with using bind and a simple free function expecting a pointer or reference to an object that should be tracked? Regards Timmo Stange
Timmo Stange wrote:
The smart pointer itself (including all temporary copies and the creation from a weak_ptr) uses atomic reference counting. Both are scalability issues (mostly the synchronization) and
No, atomic reference counting is typically not a scalability issue. "Scalability issue" means that K operations take more than K * (time for a single operation). A class-wide lock could do that, atomic increments/decrements usually do not.
exceptionally heavy in simple absolute runtime cost (as much as 50 times the cost of a simple object copy and more, depending on the CPU and system).
I know of no such CPU or system.
Peter Dimov wrote:
The smart pointer itself (including all temporary copies and the creation from a weak_ptr) uses atomic reference counting. Both are scalability issues (mostly the synchronization) and
No, atomic reference counting is typically not a scalability issue. "Scalability issue" means that K operations take more than K * (time for a single operation). A class-wide lock could do that, atomic increments/decrements usually do not.
That's why I said "mostly the synchronization" and I meant scaling with multiple threads and multiple processors.
exceptionally heavy in simple absolute runtime cost (as much as 50 times the cost of a simple object copy and more, depending on the CPU and system).
I know of no such CPU or system.
I didn't want to go into too much detail here either and therefor chose a simple combined figure for both operations. The allocation is of course more costly if it involves locking a process-wide mutex, but subsequent atomic increments roughly take 100 cycles on a Core 2 Duo and I've seen reports of higher delays with older Xeons on dual socket systems. That is all not very surprising, considering that atomic operations are essentially orthogonal to CPU designers' strategies of achieving a high instruction throughput. Regards Timmo Stange
Timmo Stange wrote:
I didn't want to go into too much detail here either and therefor chose a simple combined figure for both operations. The allocation is of course more costly if it involves locking a process-wide mutex,
It doesn't have to, given a sensible allocator.
but subsequent atomic increments roughly take 100 cycles on a Core 2 Duo and I've seen reports of higher delays with older Xeons on dual socket systems. That is all not very surprising, considering that atomic operations are essentially orthogonal to CPU designers' strategies of achieving a high instruction throughput.
Have you actually run a test and observed a 50x slowdown? You can use libs/smart_ptr/test/shared_ptr_timing_test.cpp. I'm obviously ignoring the small detail that the cleanup callback is an optional feature that appears to have no overhead unless actually used. I'm not convinced that a cleanup callback is a needed feature; just sick and tired of people citing a factor of 50 or 100 slowdown for an atomic increment. Cycle counting simply doesn't work anymore (for non-embedded CPUs); you need to measure.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 09:24 am, Peter Dimov wrote:
Timmo Stange wrote:
but subsequent atomic increments roughly take 100 cycles on a Core 2 Duo and I've seen reports of higher delays with older Xeons on dual socket systems. That is all not very surprising, considering that atomic operations are essentially orthogonal to CPU designers' strategies of achieving a high instruction throughput.
Have you actually run a test and observed a 50x slowdown? You can use libs/smart_ptr/test/shared_ptr_timing_test.cpp.
I get: fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.22 fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp -pthread fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 1.87 With boost 1.32, $ gcc --version gcc (GCC) 3.3.5 (Debian 1:3.3.5-13) on a 3.2GHz pentium d.
I'm obviously ignoring the small detail that the cleanup callback is an optional feature that appears to have no overhead unless actually used. I'm not convinced that a cleanup callback is a needed feature; just sick and tired of people citing a factor of 50 or 100 slowdown for an atomic increment. Cycle counting simply doesn't work anymore (for non-embedded CPUs); you need to measure.
- -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3v/65vihyNWuA4URArSrAKDcHszEu9Biom2vBbetXgxj3Ar4kQCfQz2j oFeoWU3IWe84FN/88fOpIfk= =CX7y -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 09:53 am, you wrote:
On Friday 23 February 2007 09:24 am, Peter Dimov wrote:
Have you actually run a test and observed a 50x slowdown? You can use libs/smart_ptr/test/shared_ptr_timing_test.cpp.
I get:
fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.22 fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp -pthread fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 1.87
With boost 1.32, $ gcc --version gcc (GCC) 3.3.5 (Debian 1:3.3.5-13) on a 3.2GHz pentium d.
For further comparison, I modified the test to use an ordinary int* instead of a shared_ptr<int>, and got: g++ -Wall -O3 raw_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.08 So it's about x3 slower using shared_ptr without -pthread, and about x20 slower using shared_ptr with -pthread. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3whF5vihyNWuA4URAjS5AKC76qY0hkrj9zTcdHlYdtVyoKgGRACfSHYG eqaS/cAm4ZJcMfbodwKsgRY= =H+i0 -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 10:29 am, you wrote:
On Friday 23 February 2007 09:53 am, you wrote:
On Friday 23 February 2007 09:24 am, Peter Dimov wrote:
Have you actually run a test and observed a 50x slowdown? You can use libs/smart_ptr/test/shared_ptr_timing_test.cpp.
I get:
fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.22 fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 shared_ptr_timing_test.cpp -pthread fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 1.87
With boost 1.32, $ gcc --version gcc (GCC) 3.3.5 (Debian 1:3.3.5-13) on a 3.2GHz pentium d.
For further comparison, I modified the test to use an ordinary int* instead of a shared_ptr<int>, and got:
g++ -Wall -O3 raw_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.08
So it's about x3 slower using shared_ptr without -pthread, and about x20 slower using shared_ptr with -pthread.
And to round it out, compiling against a boost cvs checkout gives a x10 slowdown, with the -pthread option making no difference: fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ g++ -Wall -O3 -I ~/cvs/boost shared_ptr_timing_test.cpp fhess@humidor:~/cvs/boost/libs/smart_ptr/test$ ./a.out 0.81 - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3w5q5vihyNWuA4URAr7KAKCiTALn+p0wPXCg+MCLtZJkFk0B6wCgpJPp 8xpW0DS2MxByXTHKTw4KT74= =cLYX -----END PGP SIGNATURE-----
Peter Dimov wrote:
I didn't want to go into too much detail here either and therefor chose a simple combined figure for both operations. The allocation is of course more costly if it involves locking a process-wide mutex,
It doesn't have to, given a sensible allocator.
Right, that's why the "if". We haven't discussed allocator policies for Signals, yet, but I'm not sure it will be a good idea to use one for a wide variety of different objects/sizes. I've added support for custom allocators in my implementation to be used for slot storage. I haven't yet decided if it's a good idea to rebind the same allocator for use with the target function storage (Boost.Function by default).
but subsequent atomic increments roughly take 100 cycles on a Core 2 Duo and I've seen reports of higher delays with older Xeons on dual socket systems. That is all not very surprising, considering that atomic operations are essentially orthogonal to CPU designers' strategies of achieving a high instruction throughput.
Have you actually run a test and observed a 50x slowdown? You can use libs/smart_ptr/test/shared_ptr_timing_test.cpp.
I have run tests with interlocked intrinsics on Windows/IA32. Quite naive ones, but I also once ran into actual performance problems with a project, where I could prove that excessive atomic reference counting was the bottleneck. I was wrongfully thinking it might be faster than copying objects, but it is not, at least for small objects (around 32 bytes). I even used a thread local segregated heap and intrusive counting there, btw.
I'm obviously ignoring the small detail that the cleanup callback is an optional feature that appears to have no overhead unless actually used. I'm not convinced that a cleanup callback is a needed feature; just sick and tired of people citing a factor of 50 or 100 slowdown for an atomic increment. Cycle counting simply doesn't work anymore (for non-embedded CPUs); you need to measure.
I know and I'm sorry if I gave you that impression. I did not mention cycles until you asked for more details and even then I was intentionally vague. Let's not try to get into a futile discussion about details here, because as you say, performance is not clearly predictable on modern CPU architectures anymore. I don't think micro-benchmarks are very helpful either with respect to caching (in which they give too optimistic results) and super-scalarity (where they perform worse than more complex real-life problems). This was in no way meant as a critique of shared_ptr anyway. There's no alternative to achieve the same functionality without making it too complicated for such a general use. I did not hesitate to agree on using shared_ptr as the probably best solution for thread-safe tracking with Signals and I use it for other purposes as well. It's a great tool! However, I'd like to focus on the problem at hand. I don't know how closely you have looked at Frank's thread_safe_signals, so I describe the part of the implementation I had in mind when I raised my "performance warning": Frank uses a vector of shared_ptrs to tracked objects as a member of his slot_call_iterator. That made me worry a little bit already, because I usually don't expect a temporary copy (through pass by value or other means) of an iterator to involve a vector copy and reference count adjustments for an arbitrary number of pointers. But it is a feasible way of handling the tracking and won't necessarily be a real problem for most uses. The new cleanup callback idea solely addresses the case when a slot can't be disconnected immediately, because it is being executed by another thread at the same time. If I understood Frank's new suggestion correctly, he plans to add the cleanup callback to the list of tracked objects. I'm not going to count the number of allocations this involves, but it's a lot - too much to just address something that is very unlikely to happen at all. I also see the possibility to just call the cleanup function directly (without ever copying it into a function object) in the default case. I don't care much about the exceptional case being expensive, but I don't see the reason for tracking the callback function there either. Regards Timmo Stange
Timmo Stange wrote:
However, I'd like to focus on the problem at hand. I don't know how closely you have looked at Frank's thread_safe_signals, so I describe the part of the implementation I had in mind when I raised my "performance warning":
Frank uses a vector of shared_ptrs to tracked objects as a member of his slot_call_iterator. That made me worry a little bit already, because I usually don't expect a temporary copy (through pass by value or other means) of an iterator to involve a vector copy and reference count adjustments for an arbitrary number of pointers.
The slot_call_iterator idiom is another thing that I don't like in the original Signals design; it feels too clever for its own good, and is there to support a nice way of using combiners. I've nothing against combiners per se, but the majority of the signal uses do not involve slot returns or combinations thereof. If I had to redesign the library, I'd explore a simple void operator() that just calls the slots, and a separate template<class Cm> void call_and_combine( Cm & cm, ... ); that calls cm( s( ... ) ) for each slot s. My initial iteration of this hypothetical design would also incur several shared_ptr/intrusive_ptr copies on operator() to address the thread safety issues in the most obvious "snapshot" way, that is, the signal invokes the slots that were connected at the time operator() was called. Subsequent concurrent modifications to the slot container do not affect the invocation in progress. This has the advantage of being highly scalable and deadlock-resistant. Such an approach may turn out to not be acceptable, though, given that the majority of signal benchmarks place heavy emphasis on operator() calls.
Peter Dimov wrote:
The slot_call_iterator idiom is another thing that I don't like in the original Signals design; it feels too clever for its own good, and is there to support a nice way of using combiners. I've nothing against combiners per se, but the majority of the signal uses do not involve slot returns or combinations thereof.
If I had to redesign the library, I'd explore a simple void operator() that just calls the slots, and a separate
template<class Cm> void call_and_combine( Cm & cm, ... );
that calls cm( s( ... ) ) for each slot s.
That's what the documentation describes as the "push method" and is probably also with what I had come up with as a combiner solution, mostly because I personally wouldn't use combiners that often. The complexity issue mentioned in the docs would get another aspect with thread-safe signals, because handling concurrent invocations (which your further suggestions would allow easily) won't be that simple when the combiner isn't aware of the context of a slot result.
My initial iteration of this hypothetical design would also incur several shared_ptr/intrusive_ptr copies on operator() to address the thread safety issues in the most obvious "snapshot" way, that is, the signal invokes the slots that were connected at the time operator() was called. Subsequent concurrent modifications to the slot container do not affect the invocation in progress. This has the advantage of being highly scalable and deadlock-resistant.
True. It would also simplify Frank's implementation, I think, which already uses copy-on-write semantics to handle the same problems from a different angle. As much as I like your ideas, I lack the confidence to actually move into such directions. I take the current interface as the well-con- sidered and matured design, which we should stay compatible with and build on, even though the implementation admittedly looked a little scary at first.
Such an approach may turn out to not be acceptable, though, given that the majority of signal benchmarks place heavy emphasis on operator() calls.
I sense some bitterness there ;). I think we have some freedom especially with the thread-safe implementation. Correctness and scalability always come first in that area for me. Regards Timmo Stange
Timmo Stange wrote:
Peter Dimov wrote:
The slot_call_iterator idiom is another thing that I don't like in the original Signals design; it feels too clever for its own good, and is there to support a nice way of using combiners. I've nothing against combiners per se, but the majority of the signal uses do not involve slot returns or combinations thereof.
If I had to redesign the library, I'd explore a simple void operator() that just calls the slots, and a separate
template<class Cm> void call_and_combine( Cm & cm, ... );
that calls cm( s( ... ) ) for each slot s.
Actually, a slightly better interface would be template<class Fn, class St> St call_and_combine( Fn fn, St st, ... ); that calls st = fn( st, slot( ... ) ). The cm overload could make good use of an rvalue reference; in its absence, by-value seems a bit better since it enables bind() to be passed as cm.
That's what the documentation describes as the "push method" and is probably also with what I had come up with as a combiner solution, mostly because I personally wouldn't use combiners that often.
Indeed. According to the docs, there are two downsides to the push method: the need to keep state and the inability to abort the invocation. The first is solved with using an 'accumulate-like' call_and_combine or with using bind(). int max( int x, int y ) // std::max<int> is overloaded :-/ { return x < y? y: x; } int r = signal.call_and_combine( max, INT_MIN ); // or void max( int & x, int y ) { if( x > y ) x = y; } int r = INT_MIN; signal.call_and_combine( bind( max, ref( r ), _1 ) ); The second can be solved with designating a "signal abort" exception. void max_until_K( int & x, int y, int K ) { if( x > y ) x = y; if( x > K ) throw signal_abort(); } int r = INT_MIN; signal.call_and_combine( bind( max, ref( r ), _1, 100 ) ); This allows reusing the same signal with different combiners and parameters, which is probably an academic advantage. Finally, there's always the option of just giving the users a copy of the slots and letting them obtain and combine the results at their own pace. :-)
Timmo Stange wrote:
Peter Dimov wrote:
Such an approach may turn out to not be acceptable, though, given that the majority of signal benchmarks place heavy emphasis on operator() calls.
I sense some bitterness there ;).
No, not really. I have no horse in this race. I take the benchmarks at face value; they indicate that there is obviously interest in an implementation with a fast operator(). I was never particularly fond of providing dual components with thread-safe and "thread-unsafe" interfaces (for the various definitions of that.) The ideal scenario, in my mind, is to provide a single component that is still as overhead-free as possible in the single-threaded scenario. Unfortunately, the straightforward approach of copying a vector<shared_ptr> doesn't achieve that goal, but I _think_ that it's possible to implement the same interface in a more efficient way, using copy on write and versioning tricks. Maybe Frank's implementation already does this.
On Friday 23 February 2007 07:01 pm, Peter Dimov wrote:
Timmo Stange wrote: I was never particularly fond of providing dual components with thread-safe and "thread-unsafe" interfaces (for the various definitions of that.) The ideal scenario, in my mind, is to provide a single component that is still as overhead-free as possible in the single-threaded scenario. Unfortunately, the straightforward approach of copying a vector<shared_ptr> doesn't achieve that goal, but I _think_ that it's possible to implement the same interface in a more efficient way, using copy on write and versioning tricks. Maybe Frank's implementation already does this.
Yes, it uses a shared_ptr to hold the slot list, and uses shared_ptr::unique() to decide if it really needs to make a new full copy of the slot list before adding/removing slots from the list. Signal invocations are assured no slots will be added or removed from their slot list while they are running. For a single-threaded usage pattern, the only situations it will make a new full copy in is if you call connect() or set_combiner() from a slot. -- Frank
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 17:16 pm, Peter Dimov wrote:
My initial iteration of this hypothetical design would also incur several shared_ptr/intrusive_ptr copies on operator() to address the thread safety issues in the most obvious "snapshot" way, that is, the signal invokes the slots that were connected at the time operator() was called. Subsequent concurrent modifications to the slot container do not affect the invocation in progress. This has the advantage of being highly scalable and deadlock-resistant.
Such an approach may turn out to not be acceptable, though, given that the majority of signal benchmarks place heavy emphasis on operator() calls.
Actually, since we started dropping all locks before running a slot, it has occurred to me that per-slot locking no longer buys us much concurrency-wise. Getting rid of the slot locks would require creating a list of unblocked slots at the beginning of signal invocation while holding the (only) mutex, before running any slots. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF33Rb5vihyNWuA4URAghdAKCxz1ja1Pbst1SaWGa5Ao8LRr1ANwCgtavD FncsSdKcdrBExnsangaoCww= =enpX -----END PGP SIGNATURE-----
On Thursday 22 February 2007 19:06 pm, Timmo Stange wrote:
I think we should perhaps first decide on the interface and behavior before we discuss an implementation. That cleanup callback should be optional, without parameters and return value, right? I also think it should be called synchronously directly from disconnect if the slot is not running - otherwise from the executing thread directly after the slot returns.
Yes. I suppose there is the larger issue of whether we should bother at all. The scheme I described could be implemented by the users on their own, by adding their own bogus shared_ptr with custom deleter to the list of tracked objects.
I don't directly see the necessity for a shared_ptr there and I have to express my performance worries again. As nice as shared_ptr's thread-safety is, it doesn't come for free. If you store the callback in a Boost.Function object maintained by a shared pointer, we're talking about a minimum of three heap allocations (2 for the function and 1 for the reference count) - with most allocator implementations, those allocations imply process-global synchro- nization. The smart pointer itself (including all temporary copies and the creation from a weak_ptr) uses atomic reference counting. Both are scalability issues (mostly the synchronization) and exceptionally heavy in simple absolute runtime cost (as much as 50 times the cost of a simple object copy and more, depending on the CPU and system).
I really hate to play the optimization freak here, but we should keep an eye on those things, because if the abstraction the library provides comes at a too high runtime cost, the usability is severely limited. I usually avoid heap allocations in library code and if I need to have them, I try to provide an optional allocator template.
I'm not entirely unsympathetic. When I was benchmarking thread_safe_signals, I found dynamic allocation, mutex locking, and copying shared_ptrs to all be noticeable contributors. They would increase average overhead on the order of 100 nanoseconds each. What surprised me was when I tried using fast_pool_allocator to speed up allocation of scoped_locks, it didn't help. I assume it was because fast_pool_allocator had to lock a mutex to maintain thread safety.
Could you give a short example of how that would look with using bind and a simple free function expecting a pointer or reference to an object that should be tracked?
Attached. -- Frank
On 2/23/07, Frank Mori Hess <frank.hess@nist.gov> wrote:
Could you give a short example of how that would look with using bind and a simple free function expecting a pointer or reference to an object that should be tracked?
Attached.
From the attached: boost::shared_ptr<coordinate> coord(new coordinate); coord->x = 5; sig.connect(boost::bind(&myfunction, boost::signalslib::track(coord, boost::ref(coord->x))));
Does the track() do magic within bind() such that it really has nothing to do with the signal? ie did you (or can we) build a bridge between weak_ptrs and bind/function, independent from signals? That would be really useful. Makes me start to think some of the work is being done in the wrong place - signals/slots still needs to be threadsafe, but a bind/function that handles lifetime issues would take some of the work away from signals and could be used in a lot more places. Tony
Gottlob Frege wrote:
On 2/23/07, Frank Mori Hess <frank.hess@nist.gov> wrote:
Could you give a short example of how that would look with using bind and a simple free function expecting a pointer or reference to an object that should be tracked?
Attached.
From the attached: boost::shared_ptr<coordinate> coord(new coordinate); coord->x = 5; sig.connect(boost::bind(&myfunction, boost::signalslib::track(coord, boost::ref(coord->x))));
I actually like the suggestion of just offering a way to add the tracked pointers manually. Either sig.connect( boost::bind( myfunction, boost::ref(coord->x) ), coord ); or sig.connect( boost::bind( myfunction, boost::ref(coord->x) ).track( coord ); Both look better on a syntactic level to me. In addition, I suspect that signalslib::track contains a race condition that is avoided by the other alternatives (but I haven't actually looked at it so I might be wrong; it's possible, if somewhat tedious, to implement it correctly).
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 12:22 pm, Peter Dimov wrote:
I actually like the suggestion of just offering a way to add the tracked pointers manually. Either
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ), coord );
or
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ).track( coord );
Both look better on a syntactic level to me. In addition, I suspect that signalslib::track contains a race condition that is avoided by the other alternatives (but I haven't actually looked at it so I might be wrong; it's possible, if somewhat tedious, to implement it correctly).
Would you elaborate on the race you are thinking of? Putting track in the connection class instead of the slot class makes races possible, since the connection is established before the tracked objects are added. Admittedly, this is only a problem if you are holding the tracked objects as weak_ptrs before giving them to the signal. In the slot it would be more verbose: signal_type::slot_type slot = boost::bind(&myfunction, boost::ref(coord->x)); slot.track(coord); sig.connect(slot); - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3ynY5vihyNWuA4URArIeAKDZ0snLJb4GdOnPMr4m+Si9t1wbkgCcCM2k YnbSJgwGIV/tvRxs5iHhxzQ= =O3PN -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Friday 23 February 2007 12:22 pm, Peter Dimov wrote:
I actually like the suggestion of just offering a way to add the tracked pointers manually. Either
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ), coord );
or
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ).track( coord );
Both look better on a syntactic level to me. In addition, I suspect that signalslib::track contains a race condition that is avoided by the other alternatives (but I haven't actually looked at it so I might be wrong; it's possible, if somewhat tedious, to implement it correctly).
Would you elaborate on the race you are thinking of? Putting track in the connection class instead of the slot class makes races possible, since the connection is established before the tracked objects are added. Admittedly, this is only a problem if you are holding the tracked objects as weak_ptrs before giving them to the signal.
This isn't very likely since you won't be able to use coord->x if you only have a weak_ptr. Typically you'll lock() and connect if that succeeds. The temporary shared_ptr will ensure that there is no race. This is easy to enforce if track() takes a shared_ptr instead of a weak_ptr. The singalslib::track race occurs if you have T* get_pointer( something_like_weak_ptr<T> p ) { return shared_ptr<T>( p ).get(); } There is a small window where it's possible for the weak_ptr to expire while the caller of get_pointer is holding a T*.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 13:38 pm, Peter Dimov wrote:
Frank Mori Hess wrote:
Would you elaborate on the race you are thinking of? Putting track in the connection class instead of the slot class makes races possible, since the connection is established before the tracked objects are added. Admittedly, this is only a problem if you are holding the tracked objects as weak_ptrs before giving them to the signal.
This isn't very likely since you won't be able to use coord->x if you only have a weak_ptr. Typically you'll lock() and connect if that succeeds. The temporary shared_ptr will ensure that there is no race. This is easy to enforce if track() takes a shared_ptr instead of a weak_ptr.
After I've thought about it a bit more, it is worse than that. Suppose an invocation starts before you add your shared_ptr for tracking. It will lock all the tracked weak_ptrs (but not the one you haven't added yet of course) before running the slot. Then, the thread making the new connection adds its shared_ptr for tracking and moves on, its local copy going out of scope. Then, the signal invocation continues and executes the (now possibly bad) slot without ever knowing about the tracked object. My personal preference is now near where I started out: a single-argument signals::track() function for the common case, and a track() member function of the slot class for adding an arbitrary number of arbitrary tracked objects. I don't like that the two-argument signals::track() limits the number of tracked objects to the number of bound arguments, or that it unnecessarily couples them to particular arguments.
The singalslib::track race occurs if you have
T* get_pointer( something_like_weak_ptr<T> p ) { return shared_ptr<T>( p ).get(); }
There is a small window where it's possible for the weak_ptr to expire while the caller of get_pointer is holding a T*.
That doesn't effect thread_safe_signals, since it won't run the slot except while it is already holding shared_ptrs to all the tracked objects. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF3znD5vihyNWuA4URAgD6AKDJVZk2r9aDCCFKOVe6VKE4IQKOgACeP8s0 q0oRLluCKEXyaptKsQFiO4M= =yIK8 -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Friday 23 February 2007 13:38 pm, Peter Dimov wrote:
Frank Mori Hess wrote:
Would you elaborate on the race you are thinking of? Putting track in the connection class instead of the slot class makes races possible, since the connection is established before the tracked objects are added. Admittedly, this is only a problem if you are holding the tracked objects as weak_ptrs before giving them to the signal.
This isn't very likely since you won't be able to use coord->x if you only have a weak_ptr. Typically you'll lock() and connect if that succeeds. The temporary shared_ptr will ensure that there is no race. This is easy to enforce if track() takes a shared_ptr instead of a weak_ptr.
After I've thought about it a bit more, it is worse than that. Suppose an invocation starts before you add your shared_ptr for tracking. It will lock all the tracked weak_ptrs (but not the one you haven't added yet of course) before running the slot. Then, the thread making the new connection adds its shared_ptr for tracking and moves on, its local copy going out of scope. Then, the signal invocation continues and executes the (now possibly bad) slot without ever knowing about the tracked object.
You're right, this is a problem with the second form. It can be avoided by a slight modification in the syntax: signal.track( p ).connect( bind( f, p.get() ) ); or, with some bind syntactic sugar added: signal.track( p ).connect( f, p.get() ); The slot-based formulation could look like: signal_type::slot_type( bind( f, p.get() ) ).track( p ).connect_to( signal ); or signal_type::slot_type( f, p.get() ).track( p ).connect_to( signal ); Both seem passable, the first alternative looks a bit more natural. The second has the advantage of potentially producing a reusable slot class that can operate without a signal (if we get its interface right).
On Friday 23 February 2007 14:23 pm, Peter Dimov wrote:
signal.track( p ).connect( bind( f, p.get() ) );
or, with some bind syntactic sugar added:
signal.track( p ).connect( f, p.get() );
The slot-based formulation could look like:
signal_type::slot_type( bind( f, p.get() ) ).track( p ).connect_to( signal );
or
signal_type::slot_type( f, p.get() ).track( p ).connect_to( signal );
Both seem passable, the first alternative looks a bit more natural. The second has the advantage of potentially producing a reusable slot class that can operate without a signal (if we get its interface right).
Ah, now were getting some convergence. I prefer the second, because it is nearly what I'm promoting, except you've moved the signal/connect to the end, where I would have done: signal.connect(signal_type::slot_type( f, p.get() ).track( p )); However, my way also allows track() to be called an arbitrary number of times before connecting the slot. How would multiple tracked objects be handled with a track(p).connect()/.connect_to() syntax? If you allow track to take varying numbers of arguments, it requires the user to know how many tracked objects there will be at compile time. Being able to call track() multiple times only requires knowing the number of tracked objects at run time, which might be handy if the tracked objects are being collected in a container. I suppose you could overload track to accept a container argument. Also, support for the first alternative could be easily implemented using the second, since whatever object is returned by signal::track() will have both a reference to the signal and the slot in its connect() function. -- Frank
Frank Mori Hess wrote:
However, my way also allows track() to be called an arbitrary number of times before connecting the slot. How would multiple tracked objects be handled with a track(p).connect()/.connect_to() syntax?
I suppose track() would return a reference to the slot_type, so that you could do: signal_type::slot_type( f, p1.get(), p2.get() ).track( p1 ).track( p2 ).connect_to(...);
If you allow track to take varying numbers of arguments, it requires the user to know how many tracked objects there will be at compile time. Being able to call track() multiple times only requires knowing the number of tracked objects at run time, which might be handy if the tracked objects are being collected in a container. I suppose you could overload track to accept a container argument.
Also, support for the first alternative could be easily implemented using the second, since whatever object is returned by signal::track() will have both a reference to the signal and the slot in its connect() function.
Another question is if both alternatives should be provided, or if tracking should only work when you create a slot_type object explicitly. I'd also like to mention that you can effectively hide the tracking from the connection setup site with the original implementation. If you only provide a typedef for the functor you want to be used for the connection, visit_each can collect the information automatically (or at least in a way invisible to the connecting code). Regards Timmo Stange
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 16:50 pm, Timmo Stange wrote:
Frank Mori Hess wrote:
However, my way also allows track() to be called an arbitrary number of times before connecting the slot. How would multiple tracked objects be handled with a track(p).connect()/.connect_to() syntax?
I suppose track() would return a reference to the slot_type, so that you could do:
signal_type::slot_type( f, p1.get(), p2.get() ).track( p1 ).track( p2 ).connect_to(...);
Ah, yes. track() has to return a reference to the slot for my suggestion signal.connect(signal_type::slot_type( f, p.get() ).track( p )); to work as well.
Another question is if both alternatives should be provided, or if tracking should only work when you create a slot_type object explicitly.
I have no problem with only providing tracking support through slot::track().
I'd also like to mention that you can effectively hide the tracking from the connection setup site with the original implementation. If you only provide a typedef for the functor you want to be used for the connection, visit_each can collect the information automatically (or at least in a way invisible to the connecting code).
I'm not following what you're saying here. Can you give some kind of example for what you're suggesting? - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF32Qp5vihyNWuA4URAnrrAJwJNzqb0zzQrBojAdK4SfT7L1+d/ACfYtIB Cj8GatxyOXCgO7TBzdCpuu0= =diYH -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
Another question is if both alternatives should be provided, or if tracking should only work when you create a slot_type object explicitly.
I have no problem with only providing tracking support through slot::track().
I agree.
I'd also like to mention that you can effectively hide the tracking from the connection setup site with the original implementation. If you only provide a typedef for the functor you want to be used for the connection, visit_each can collect the information automatically (or at least in a way invisible to the connecting code).
I'm not following what you're saying here. Can you give some kind of example for what you're suggesting?
You can look at random_signal_system.cpp for a real example, but it's quite simple: A class can provide a predefined function class and an overload for visit_each. The connecting code creates an instance of that class to connect to the signal and doesn't need to care about tracking, because it is set up by the slot and visit_each. struct some_observer { signals::tracked ptr_to_some_secret_object; some_observer(); // Sets up ptr_to_some_secret_object void operator()(); // Uses ptr_to_some_secret_object }; ... signal0<void> sig; sig.connect(some_observer()); The advantage is that the party establishing the connection does not need to know what objects have to be tracked. It's not such a big asset, but it's something you cannot do with the explicit track(). Regards Timmo Stange
Timmo Stange wrote:
struct some_observer { signals::tracked ptr_to_some_secret_object; some_observer(); // Sets up ptr_to_some_secret_object void operator()(); // Uses ptr_to_some_secret_object };
...
signal0<void> sig; sig.connect(some_observer());
The advantage is that the party establishing the connection does not need to know what objects have to be tracked. It's not such a big asset, but it's something you cannot do with the explicit track().
Something along the lines of: template<class Sig> class slot { private: shared_ptr< slot_impl<Sig> > pi_; public: template<class F> slot( F f ): pi_( new slot_impl<Sig>( f ) ) {} result_type operator()( ... ) { return (*pi_)( ... ) } template<class T> void track( shared_ptr<T> pt ) { pi_->track( pt ); } }; template<class Sig> class signal { private: std::vector< slot<Sig> > slots_; mutable mutex mx_; public: // illustration only void operator()( ... ) const { scoped_lock lock( mx_ ); std::vector< slot<Sig> > tmp( slots_ ); lock.unlock(); for_each( tmp.begin(), tmp.end(), bind( &slot<Sig>::operator(), _1, ... ) ); } }; allows your example to be restated as: slot<void()> make_observer_slot( ... ) { return slot<void()>( some_observer( ... ) ).track( whatever ); } // ... signal<void()> sig; sig.connect( make_observer_slot() ); with some_observer no longer needing to have a dependency on signals.
Peter Dimov wrote:
Something along the lines of:
template<class Sig> class slot { private:
shared_ptr< slot_impl<Sig> > pi_;
public:
template<class F> slot( F f ): pi_( new slot_impl<Sig>( f ) ) {} result_type operator()( ... ) { return (*pi_)( ... ) } template<class T> void track( shared_ptr<T> pt ) { pi_->track( pt ); } };
template<class Sig> class signal { private:
std::vector< slot<Sig> > slots_; mutable mutex mx_;
public:
// illustration only
void operator()( ... ) const { scoped_lock lock( mx_ ); std::vector< slot<Sig> > tmp( slots_ ); lock.unlock();
for_each( tmp.begin(), tmp.end(), bind( &slot<Sig>::operator(), _1, .... ) ); } };
allows your example to be restated as:
slot<void()> make_observer_slot( ... ) { return slot<void()>( some_observer( ... ) ).track( whatever ); }
// ...
signal<void()> sig; sig.connect( make_observer_slot() );
with some_observer no longer needing to have a dependency on signals.
Hm, I think that still bears a higher level of dependency between connection site and observer - this time the observer needs to know the signal's signature. While you can use bind(...) on an ordinary function class, it would be difficult to forward the tracking to the true slot (I'd call the independently created one a slot model) after binding. Regards Timmo Stange
On Friday 23 February 2007 06:26 pm, Timmo Stange wrote:
Hm, I think that still bears a higher level of dependency between connection site and observer - this time the observer needs to know the signal's signature. While you can use bind(...) on an ordinary function class, it would be difficult to forward the tracking to the true slot (I'd call the independently created one a slot model) after binding.
Couldn't tracked objects could be copied from one slot to another by overloading slot::track to accept a slot as an argument? Then binding a slot to another slot with a different signature would might look like signal.connect(signal_type::slot_type(slot, _2, _1).track(slot)); It's a bit murky whether track(slot) would only cause the tracked objects of slot to get tracked, or also track the slot itself (or its pimpl). I suppose it would depend on whether copies of slots are deep or share the same SlotFunction. -- Frank
Frank Mori Hess wrote:
Couldn't tracked objects could be copied from one slot to another by overloading slot::track to accept a slot as an argument? Then binding a slot to another slot with a different signature would might look like
signal.connect(signal_type::slot_type(slot, _2, _1).track(slot));
It's a bit murky whether track(slot) would only cause the tracked objects of slot to get tracked, or also track the slot itself (or its pimpl). I suppose it would depend on whether copies of slots are deep or share the same SlotFunction.
You don't need tracking for the above example; bind( slot, _2, _1 ) stores a copy of slot. Signals currently need tracking because they're noncopyable and are bound using ref(). A copyable signal does not, regardless of the semantics of the copy (deep, shallow, or something in between - deep for the signal, shallow for the contained slots).
Peter Dimov wrote:
Frank Mori Hess wrote:
Couldn't tracked objects could be copied from one slot to another by overloading slot::track to accept a slot as an argument? Then binding a slot to another slot with a different signature would might look like
signal.connect(signal_type::slot_type(slot, _2, _1).track(slot));
It's a bit murky whether track(slot) would only cause the tracked objects of slot to get tracked, or also track the slot itself (or its pimpl). I suppose it would depend on whether copies of slots are deep or share the same SlotFunction.
You don't need tracking for the above example; bind( slot, _2, _1 ) stores a copy of slot.
Now that I think about it, it can be implemented both ways. A slot that requires an explicit lock() before operator() would still need the .track. A slot that throws bad_weak_ptr from operator() would not. This brings us back to the problem of autodisconnects only happening on calls. :-/ It doesn't have to be either-or, though.
On Sunday 25 February 2007 12:07 pm, Peter Dimov wrote:
Now that I think about it, it can be implemented both ways. A slot that requires an explicit lock() before operator() would still need the .track. A slot that throws bad_weak_ptr from operator() would not. This brings us back to the problem of autodisconnects only happening on calls. :-/ It doesn't have to be either-or, though.
Yes, I was assuming an explicit lock(), and perhaps exposing the tracked object list read-only for efficiency, which would allow multiple slots to be locked at once without creating a separate container for each slot. -- Frank
Frank Mori Hess wrote:
On Sunday 25 February 2007 12:07 pm, Peter Dimov wrote:
Now that I think about it, it can be implemented both ways. A slot that requires an explicit lock() before operator() would still need the .track. A slot that throws bad_weak_ptr from operator() would not. This brings us back to the problem of autodisconnects only happening on calls. :-/ It doesn't have to be either-or, though.
Yes, I was assuming an explicit lock(), and perhaps exposing the tracked object list read-only for efficiency, which would allow multiple slots to be locked at once without creating a separate container for each slot.
I think I prefer: bool expired() const; R operator()( A... ) const; // throws bad_weak_ptr operator() could do alloca() tricks to avoid dynamic allocations. The portable version could use a local array< shared_ptr, K > to catch the common case when the number of tracked objects is no more than K. Hiding a slot behind a bind() or some other adaptor (std::not1, say) would make it invisible to the expired() query, but it will still track its objects within its operator().
On Sunday 25 February 2007 12:39 pm, Peter Dimov wrote:
I think I prefer:
bool expired() const; R operator()( A... ) const; // throws bad_weak_ptr
As I recall, the other objection raised by Timmo was the ambiguity as to whether a caught bad_weak_ptr exception was thrown by the slot or the underlying SlotFunction. In one case, the signal would just disconnect the slot and move on. In the other, it would want to rethrow the exception. Perhaps that could be mitigated by throwing a more specific exception, like an expired_slot derived from bad_weak_ptr? -- Frank
Frank Mori Hess wrote:
On Sunday 25 February 2007 12:39 pm, Peter Dimov wrote:
I think I prefer:
bool expired() const; R operator()( A... ) const; // throws bad_weak_ptr
As I recall, the other objection raised by Timmo was the ambiguity as to whether a caught bad_weak_ptr exception was thrown by the slot or the underlying SlotFunction. In one case, the signal would just disconnect the slot and move on. In the other, it would want to rethrow the exception.
Why would you want to rethrow a bad_weak_ptr exception? Its meaning is unambiguous; a weak_ptr has expired. This SlotFunction is most probably dead and is informing you of its sorry state.
On Sunday 25 February 2007 01:37 pm, Peter Dimov wrote:
Frank Mori Hess wrote:
On Sunday 25 February 2007 12:39 pm, Peter Dimov wrote:
I think I prefer:
bool expired() const; R operator()( A... ) const; // throws bad_weak_ptr
As I recall, the other objection raised by Timmo was the ambiguity as to whether a caught bad_weak_ptr exception was thrown by the slot or the underlying SlotFunction. In one case, the signal would just disconnect the slot and move on. In the other, it would want to rethrow the exception.
Why would you want to rethrow a bad_weak_ptr exception? Its meaning is unambiguous; a weak_ptr has expired. This SlotFunction is most probably dead and is informing you of its sorry state.
Okay, but the other issue is the slot iterator has to know that the slot has not (and will not) expire before calling it. This means an explicit lock(), unless we commit to discarding the current combiner interface. -- Frank
On Sunday 25 February 2007 11:42 am, Peter Dimov wrote:
signal.connect(signal_type::slot_type(slot, _2, _1).track(slot));
It's a bit murky whether track(slot) would only cause the tracked objects of slot to get tracked, or also track the slot itself (or its pimpl). I suppose it would depend on whether copies of slots are deep or share the same SlotFunction.
You don't need tracking for the above example; bind( slot, _2, _1 ) stores a copy of slot. Signals currently need tracking because they're noncopyable and are bound using ref(). A copyable signal does not, regardless of the semantics of the copy (deep, shallow, or something in between - deep for the signal, shallow for the contained slots).
But you need to at least copy the list of tracked objects from the slot inside the bind to the slot received by connect(). Or, you would have to use visit each to discover any slots inside the bind and get their tracked object lists. -- Frank
Peter Dimov wrote:
You don't need tracking for the above example; bind( slot, _2, _1 ) stores a copy of slot. Signals currently need tracking because they're noncopyable and are bound using ref(). A copyable signal does not, regardless of the semantics of the copy (deep, shallow, or something in between - deep for the signal, shallow for the contained slots).
How would such a copy behave regarding connect() and disconnect()? Neither should affect the original signal, so this would be either a deep copy or an implementation with copy on write semantics. I already don't like the fact that there is no distinction between control over connections and control over invocation. Every entity that is allowed to connect to a signal can also emit it, unless you provide a separate connect() yourself and hide the actual signal. In that case only the signal's owner can connect it to another signal and the whole tracking idea becomes somewhat uninteresting. Is a shared signal (in the sense of a shared_ptr) common practice? Regards Timmo Stange
Timmo Stange wrote:
Peter Dimov wrote:
You don't need tracking for the above example; bind( slot, _2, _1 ) stores a copy of slot. Signals currently need tracking because they're noncopyable and are bound using ref(). A copyable signal does not, regardless of the semantics of the copy (deep, shallow, or something in between - deep for the signal, shallow for the contained slots).
How would such a copy behave regarding connect() and disconnect()? Neither should affect the original signal, so this would be either a deep copy or an implementation with copy on write semantics.
A shared signal is mostly equivalent to the current noncopyable signal, except that it's less inconvenient and less unsafe. Currently you are forced to use references or pointers to the signal, and every entity that posesses such a reference can connect() and disconnect(). A shared_ptr<signal_impl> reformulation has the same effect, minus the possibility of a dangling reference. The next step is to have a shallow slot (shared_ptr<slot_impl>) but a one level deep copy in the signal (vector<slot>). In this case connects and disconnects will only affect the copy, although the sharing at the SlotFunction level may still be undesirable at times. Immutable SlotFunctions would handle sharing (and concurrent calls) fine, of course; only stateful function objects will have a problem. A fully deep copy (vector< function<> >) avoids any sharing, but makes the signal quite expensive to copy.
I already don't like the fact that there is no distinction between control over connections and control over invocation. Every entity that is allowed to connect to a signal can also emit it, unless you provide a separate connect() yourself and hide the actual signal.
Is this a problem in practice?
Peter Dimov wrote:
How would such a copy behave regarding connect() and disconnect()? Neither should affect the original signal, so this would be either a deep copy or an implementation with copy on write semantics.
A shared signal is mostly equivalent to the current noncopyable signal, except that it's less inconvenient and less unsafe. Currently you are forced to use references or pointers to the signal, and every entity that posesses such a reference can connect() and disconnect(). A shared_ptr<signal_impl> reformulation has the same effect, minus the possibility of a dangling reference.
A signal would then rather be a handle object, or not? I wouldn't see a problem in this if it were just for the invocation and connection to another signal, but with member functions like connect() and disconnect() it would appear like a rather unusual construct to me.
The next step is to have a shallow slot (shared_ptr<slot_impl>) but a one level deep copy in the signal (vector<slot>). In this case connects and disconnects will only affect the copy, although the sharing at the SlotFunction level may still be undesirable at times. Immutable SlotFunctions would handle sharing (and concurrent calls) fine, of course; only stateful function objects will have a problem.
A fully deep copy (vector< function<> >) avoids any sharing, but makes the signal quite expensive to copy.
When the user provided a reference to a function object for the slot construction, the deep copy will still not give you a function with its own state. Together with connects and disconnects being only effective on one copy, this can become quite messy. What happens to the connection objects returned by connect()? Do they only refer to the original slot or also to every copy?
I already don't like the fact that there is no distinction between control over connections and control over invocation. Every entity that is allowed to connect to a signal can also emit it, unless you provide a separate connect() yourself and hide the actual signal.
Is this a problem in practice?
No, not really. I was only reminded of this by the idea of copyable signals. It would be easy to split a signal into a connection managing base and invocable subclass, so that the owner could just expose a reference to the base for observers to connect to. I couldn't come up with a proper name for that base class right now, though. Regards Timmo Stange
On Sunday 25 February 2007 07:40 pm, Timmo Stange wrote:
A signal would then rather be a handle object, or not? I wouldn't see a problem in this if it were just for the invocation and connection to another signal, but with member functions like connect() and disconnect() it would appear like a rather unusual construct to me.
It would be like signals::connection, which is a handle object and has a disconnect(). -- Frank
Frank Mori Hess wrote:
A signal would then rather be a handle object, or not? I wouldn't see a problem in this if it were just for the invocation and connection to another signal, but with member functions like connect() and disconnect() it would appear like a rather unusual construct to me.
It would be like signals::connection, which is a handle object and has a disconnect().
Right, but that's a much clearer concept in my book. The connection describes a relation between the signal and a slot and nothing concrete that can live on its own. Let me try to explain it from a different angle: What good does this copyable signal handle do? You don't need to track it, because you have a full copy (that's where the idea originated from). That means you can still receive notifications after the original object which you decided to observe has long gone. You can still connect new slots to the signal to be notified of state changes in that dead entity ;). I may be missing some important practical use (that's why I asked about common practice with shared - or perhaps independent - signals), but at the moment it looks more like a design choice that is based on implementation considerations to me. Regards Timmo Stange
Timmo Stange wrote:
Let me try to explain it from a different angle: What good does this copyable signal handle do?
Same as all C++ handles; prevent dangling references. C++ handles are the equivalent of Java references. Value semantics are better when they're possible, but if the alternative is noncopyability and raw pointers, I always prefer the handle approach. Not all C++ programmers agree. A construct on which Java is built upon must be evil. :-) That said, in this case I think that the hybrid approach is better than the two other alternatives.
Peter Dimov wrote:
Let me try to explain it from a different angle: What good does this copyable signal handle do?
Same as all C++ handles; prevent dangling references. C++ handles are the equivalent of Java references. Value semantics are better when they're possible, but if the alternative is noncopyability and raw pointers, I always prefer the handle approach. Not all C++ programmers agree. A construct on which Java is built upon must be evil. :-)
I think there is a much more valid objection: as you state yourself, it's not a natural C++ language construct. I prefer to give handles a distinctive name (..._handle, ..._pointer, ..._proxy, etc.) to indi- cate that this is not the real object with value semantics. In the future we well thankfully have shared_ptr, which will be well noticed as what it is by a C++ programmer. I think it would suffice here, too, *if* we need this.
That said, in this case I think that the hybrid approach is better than the two other alternatives.
I would like to get a little bit more concrete here: For what do we need this? Is the signal really something a client should store references to? In a case of a real independent signal that is not owned by a single entity, but shared by several, is a shared_ptr not the better alternative? I still only see the signal-to-signal connection as the most probable case in which a reference to the signal needs to be stored somewhere and I think such a connection would usually be established by the signal owner, which makes tracking simple or completely unnecessary and the binding to the signal (C++-)reference is not a problem at all. Regards Timmo Stange
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 17:26 pm, Timmo Stange wrote:
You can look at random_signal_system.cpp for a real example, but it's quite simple: A class can provide a predefined function class and an overload for visit_each. The connecting code creates an instance of that class to connect to the signal and doesn't need to care about tracking, because it is set up by the slot and visit_each.
struct some_observer { signals::tracked ptr_to_some_secret_object; some_observer(); // Sets up ptr_to_some_secret_object void operator()(); // Uses ptr_to_some_secret_object };
...
signal0<void> sig; sig.connect(some_observer());
The advantage is that the party establishing the connection does not need to know what objects have to be tracked. It's not such a big asset, but it's something you cannot do with the explicit track().
Ah, I see. Couldn't you achieve the same thing by deriving some_observer from the slot class? Then the some_observer constructor could call slot::track() to set up its own tracking. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF33AR5vihyNWuA4URAuBEAKDZHKwRKBdIW/xsBTkt8SHrphb+JwCgjQWk Hxs/2P3ulFzNPnl/PVdTZR0= =K/GW -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
Ah, I see. Couldn't you achieve the same thing by deriving some_observer from the slot class? Then the some_observer constructor could call slot::track() to set up its own tracking.
The target function class would need to know the signal's slot_type for that, which is not really the same situation we have with implicit tracking. It would work though. I'm actually undecided here. I don't really like providing both al- ternatives, because it makes the interface confusing, as I've stated earlier. I now see that using the implicit tracking alone will lead to syntactically obscure constructs with the non-trivial cases (tracking of not directly related objects). The explicit tracking solves that and also removes a lot of behind-the-scenes magic. If the latter becomes the only solution, I'd prefer it being applicable on connections, too, though. The explicit construction of slots is otherwise seldom necessary and I wasn't aware it is at all possible before I started working on signals. We should also keep in mind what the tracking will be used for mostly. I think this is in order of importance: 1. tracking of objects for member function slots 2. tracking of parameters directly 3. tracking of signals as slots 4. tracking of indirectly related objects If that is close to reality, shifting to explicit tracking is a major change. Cases 1 and 2 are handled quite well with the implicit tracking. Case 3 involves some acrobatics in the implementation and case 4 leads to an obscure syntax. On the other hand, all cases will result in two or more statements with explicit tracking. I find it difficult to favor one or the other here. Regards Timmo Stange
Timmo Stange wrote:
If the latter becomes the only solution, I'd prefer it being applicable on connections, too, though. The explicit construction of slots is otherwise seldom necessary and I wasn't aware it is at all possible before I started working on signals.
Adding track() to connections is prone to race conditions, though. Regards Timmo Stange
On Saturday 24 February 2007 10:10 am, Timmo Stange wrote:
We should also keep in mind what the tracking will be used for mostly. I think this is in order of importance: 1. tracking of objects for member function slots 2. tracking of parameters directly 3. tracking of signals as slots 4. tracking of indirectly related objects
If that is close to reality, shifting to explicit tracking is a major change. Cases 1 and 2 are handled quite well with the implicit tracking. Case 3 involves some acrobatics in the implementation
I wasn't planning on dropping support for automatic tracking of signals, I don't see that dropping it would make things any less confusing for the user.
and case 4 leads to an obscure syntax. On the other hand, all cases will result in two or more statements with explicit tracking. I find it difficult to favor one or the other here.
signal.connect(signal_type::slot_type( f, p.get() ).track( p )); is only 1 statement. True, it's a more complicated expression than signal.connect(f, boost::signals::track(p))); -- Frank
On Saturday 24 February 2007 11:18 am, Frank Mori Hess wrote:
On Saturday 24 February 2007 10:10 am, Timmo Stange wrote:
We should also keep in mind what the tracking will be used for mostly. I think this is in order of importance: 1. tracking of objects for member function slots 2. tracking of parameters directly 3. tracking of signals as slots 4. tracking of indirectly related objects
If that is close to reality, shifting to explicit tracking is a major change. Cases 1 and 2 are handled quite well with the implicit tracking. Case 3 involves some acrobatics in the implementation
I wasn't planning on dropping support for automatic tracking of signals, I don't see that dropping it would make things any less confusing for the user.
Or, rather, I'd say if automatic signal tracking is unintuitive, that happened back when we deprecated trackable, with its idea of trackability being a property of a class. -- Frank
Frank Mori Hess wrote:
I wasn't planning on dropping support for automatic tracking of signals, I don't see that dropping it would make things any less confusing for the user.
Automatic tracking is nice, but is it workable, if the goal is for the 'new' signals to become part of the C++ standard library one day? It requires cooperation from the function objects, and this is hard to achieve in practice. (I'm not sure of the status of the TR2 signal proposal.)
On Saturday 24 February 2007 11:59 am, Peter Dimov wrote:
Frank Mori Hess wrote:
I wasn't planning on dropping support for automatic tracking of signals, I don't see that dropping it would make things any less confusing for the user.
Automatic tracking is nice, but is it workable, if the goal is for the 'new' signals to become part of the C++ standard library one day? It requires cooperation from the function objects, and this is hard to achieve in practice.
Hmm, well I suppose it wouldn't be too bad if we just provided an overloaded version slot::track that accepted a signal as an argument. -- Frank
Peter Dimov wrote:
I wasn't planning on dropping support for automatic tracking of signals, I don't see that dropping it would make things any less confusing for the user.
Automatic tracking is nice, but is it workable, if the goal is for the 'new' signals to become part of the C++ standard library one day? It requires cooperation from the function objects, and this is hard to achieve in practice.
It's questionable if the whole idea of tracking through shared_ptr is viable for a standardization effort. I of course have no experience with this, but seeing what has been accepted into TR1, I'm a bit sceptical about a TR2 signals and similarly "rich" functionality. It would probably be better to not depend on the tracking anywhere, so that it can be dropped altogether, if it turns out impossible to come to an agreement. This would mean we get rid of the specialized handling of signals as a slot with automatic tracking (it comes cheaply with the pimpl idiom, but that's an implementation detail). If the user does not manage the connection and signal lifetime externally, they will have to use track(). That's not a big problem in my opinion. There is a bit of an ambivalence here, since we're still trying to maintain compatibility to the current Boost.Signals. I would also like to make the slot templated by the call signature instead of the storage type (that's what Boost.Signals does) as you implied in one of your examples. Otherwise the easiest way of determining the slot type is through typename signal_type::slot_type which is perhaps too inconvenient in that the exact signal type must be known. Regards Timmo Stange
Frank Mori Hess wrote:
However, my way also allows track() to be called an arbitrary number of times before connecting the slot. How would multiple tracked objects be handled with a track(p).connect()/.connect_to() syntax? If you allow track to take varying numbers of arguments, it requires the user to know how many tracked objects there will be at compile time. Being able to call track() multiple times only requires knowing the number of tracked objects at run time, which might be handy if the tracked objects are being collected in a container.
This is a good argument for the slot-based alternative. slot_type slot( f, ... ); for_each( tracked.begin(), tracked.end(), bind( &slot_type::track, _1 ) ); signal.connect( slot ); // or slot.connect_to( signal );
I suppose you could overload track to accept a container argument.
This would work well with the signal.connect( bind( f, ... ), tracked ); syntax, but we're losing the ability to drop the bind. :-)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 17:06 pm, Peter Dimov wrote:
slot_type slot( f, ... );
for_each( tracked.begin(), tracked.end(), bind( &slot_type::track, _1 ) );
Sorry for being pedantic here, but I think you left the "slot" argument out of the bind call? bind( &slot_type::track, slot, _1 ) - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF32v25vihyNWuA4URAsqIAKC2d2fGdwJN5quQPWzv6++20LwSbgCfUA/s OwNo1eGzsAJSINspb1JAqhM= =+h2w -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Friday 23 February 2007 17:06 pm, Peter Dimov wrote:
slot_type slot( f, ... );
for_each( tracked.begin(), tracked.end(), bind( &slot_type::track, _1 ) );
Sorry for being pedantic here, but I think you left the "slot" argument out of the bind call?
bind( &slot_type::track, slot, _1 )
You're right, I did, but we all knew what I meant. :-) As long as we're in pedantic mode: bind( &slot_type::track, &slot, _1 )
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 17:49 pm, Peter Dimov wrote:
Frank Mori Hess wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Friday 23 February 2007 17:06 pm, Peter Dimov wrote:
slot_type slot( f, ... );
for_each( tracked.begin(), tracked.end(), bind( &slot_type::track, _1 ) );
Sorry for being pedantic here, but I think you left the "slot" argument out of the bind call?
bind( &slot_type::track, slot, _1 )
You're right, I did, but we all knew what I meant. :-) As long as we're in pedantic mode:
bind( &slot_type::track, &slot, _1 )
Well, if I was _really_ pedantic, I'd remind you that the clever designer of mem_fn made the following allowance: "the returned function object can take a pointer, a reference, or a smart pointer to an object instance as its first argument" Fortunately, I'm above that sort of thing :) - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF33Oa5vihyNWuA4URAhWlAJ4mboMFIbOFfq23whgmtu9ff42g+ACcDzTT 7g3uVB2n+TTRhx9bSKvfXpQ= =z3WY -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 23 February 2007 18:07 pm, you wrote:
bind( &slot_type::track, slot, _1 )
You're right, I did, but we all knew what I meant. :-) As long as we're in pedantic mode:
bind( &slot_type::track, &slot, _1 )
Well, if I was _really_ pedantic, I'd remind you that the clever designer of mem_fn made the following allowance:
"the returned function object can take a pointer, a reference, or a smart pointer to an object instance as its first argument"
Fortunately, I'm above that sort of thing :)
Or does it not work after all, due to binding to a copy of the slot object? - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFF33XN5vihyNWuA4URAt0bAJ93OylELR/38Q5LZCBihPe5v3ynGACfZAq/ AJeqvikLUCXRYT4Zt22IOco= =XPsR -----END PGP SIGNATURE-----
On 2/23/07, Peter Dimov <pdimov@mmltd.net> wrote:
I actually like the suggestion of just offering a way to add the tracked pointers manually. Either
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ), coord );
or
sig.connect( boost::bind( myfunction, boost::ref(coord->x) ).track( coord );
Both look better on a syntactic level to me. In addition, I suspect that signalslib::track contains a race condition that is avoided by the other alternatives (but I haven't actually looked at it so I might be wrong; it's possible, if somewhat tedious, to implement it correctly).
That is what I was expecting it to look like, but don't you think a solution that goes beyond signals/slots would be beneficial (if it is possible to do)? I've used bind/function in threads before (managing or knowing the lifetime independently) OUTSIDE of signals/slots, so I'd love to see the more general solution. I know I've seen people asking for boost bind/function to work with weak_ptrs. Tony
Gottlob Frege wrote:
That is what I was expecting it to look like, but don't you think a solution that goes beyond signals/slots would be beneficial (if it is possible to do)? I've used bind/function in threads before (managing or knowing the lifetime independently) OUTSIDE of signals/slots, so I'd love to see the more general solution.
I know I've seen people asking for boost bind/function to work with weak_ptrs.
It would make quite a bit of sense sense for bind/mem_fn to accept a weak_ptr as the object argument and throw a bad_weak_ptr if it expires. This was my favorite approach of replacing the current tracking mechanism since I'm a devoted fan of the KISS principle. Relying on this alone was considered not enough, though. The reasons that've been cited were (1) that one could reasonably want to use a weak_ptr or a tracked pointer as another argument, and (2) that delaying the automatic disconnect until the slot is actually called may consume unacceptably large amounts of memory in scenarios where objects are frequently connected and then exprire, while signal calls are rare.
participants (1)
-
Frank Mori Hess -
Frank Mori Hess -
Gottlob Frege -
Peter Dimov -
Timmo Stange