On Sat, 2008-04-12 at 01:10 +0300, Peter Dimov wrote:

Daniel Frey:

...

I attached a complete implementation of enable_shared_from_this_light which has an overhead of a single plain pointer. [...]

I agree that providing the ability to build this lightweight helper may be valuable to some. But I'm not sure that we should provide several esft classes in Boost (except possibly as examples). This of course raises the question of which esft base (of the three candidates we have) should be provided.

I think that we should first find out which generally useful helper functions shared_ptr/weak_ptr should have and how we can improve the framework. It seems to me that access to the shared_count/weak_count members might be useful, as well as the new ctor(s) (see below).

...

The thing to note is that I added two more things to shared_ptr: _internal_shared_count() and a ctor which takes a detail::weak_count and the raw pointer.

I'd change the first argument of the new constructor to detail::shared_count.

Wouldn't that lead to an additional reference counter increment/decrement cycle? Maybe we should add a ctor which takes a shared_count in addition to the one that takes a weak_count. The ctor which takes the shared_count might then be used to replace the ctor with the ignore_enable_shared_from_this_tag?

I don't like your preferred spelling of the function. It doesn't reflect its semantics in my opinion. In shared_ptr<T> p( new T ), the T object is not an observer, but it does accept p as its owner.

To me "accept_owner" communicates that the callee has to make some kind of decision of whether or not it accepts the ownership, which I find misleading. My point of view is that T just observes when a shared_ptr takes ownership and is notified as soon as it happened. But then, since I'm not a native English speaker, that doesn't mean much :) Regards, Daniel

On Sat, 2008-04-12 at 03:29 +0300, Peter Dimov wrote:

It won't if we use

shared_ptr( detail::shared_count pn, Y* px );

and swap(pn,pn_) as poor man's move.

I updated the patch to replace ignore_enable_shared_from_this_tag with this approach. Which means that shared_ptr no longer contains any traces of enable_shared_from_this[_light], only some generally useful extensions, which is IMHO a good sign. I also replaced the name sp_notify_observer with sp_accept_owner.

Let me bring up something else. Should

shared_ptr(Y* p, D d);

call sp_accept_owner(this, p), or should it call sp_accept_owner(this, p, &d'), where d' is the stored deleter?

Since the pointer to the deleter could always be retrieved by sp_accept_owner via the first parameter, the only benefit is that the deleter's type D is passed, right? To me it seems cleaner if sp_accept_owner should not have any direct link to the way *how* the shared_ptr was created, just the fact that it has been created. IMHO it should says: "This shared_pointer now owns this object" and not "This shared_ptr now owns this object and was created with this deleter". I simply fail to see what this should be good for, but maybe you have a use case in mind? Regards, Daniel

Frank Mori Hess:

I'd say go with the 1.35-like implementation. I think using sp_accept_owner is preferable to shared_from_this in constructors, due to the exception concerns I posted about earlier:

http://article.gmane.org/gmane.comp.lib.boost.devel/173097

So you suggest we throw your improved implementation away? :-) This doesn't seem right to me, as it represents a fair amount of knowledge that will be lost. Maybe we need to move it somewhere, to boost/smart_ptr/esft_constructor_base.hpp, for instance? And link it from the docs.

Frank Mori Hess:

On Saturday 12 April 2008 12:07 pm, Peter Dimov wrote:

...

So you suggest we throw your improved implementation away? :-) This doesn't seem right to me, as it represents a fair amount of knowledge that will be lost. Maybe we need to move it somewhere, to boost/smart_ptr/esft_constructor_base.hpp, for instance? And link it from the docs.

Keeping it (and the enable_shared_from_this_light) as example code seems fine.

While working on the sp_accept_owner change: http://svn.boost.org/trac/boost/changeset/44353 I was reminded that the enhanced esft base actually can't be done in "user mode" due to the get_deleter support. So it's not just an example; if we keep it, it has to be "official". I leave the decision (and the actual revert, should you elect to proceed) to you. It might be better to wait for a test cycle first. I'll also appreciate if you try sp_accept_owner for your existing use cases that motivated the enhanced esft base and, if possible, distill their essence into a test case that will replace (or complement) esft_constructor_test. As an aside, if people interested in the Borland compiler are listening, I have problems with sp_accept_owner_test.cpp on bcc32 5.5.1. It doesn't seem to recognize my sp_accept_owner overloads. It'd be somewhat disturbing if this persists for the later Borland versions which we test. Oddly, shared_from_this_test and esft_regtest appear to work fine, and I've no idea why.

Daniel Frey:

On Sat, 2008-04-12 at 21:37 +0300, Peter Dimov wrote:

...

http://svn.boost.org/trac/boost/changeset/44353

This introduces overhead to both ctors which take a deleter, they are now calling get_deleter in order to pass it to sp_accept_owner.

Yes, it does. This is an implementation artifact. The Overhead(tm) can easily (if not particularly elegantly) be avoided by piercing the shared_count abstraction a bit, if one gets motivated enough by an extra virtual call.

This is IMHO not needed, the called sp_accept_owner can call get_deleter on the shared_ptr itself.

If passing the type of the deleter itself is really needed, it can be done without overhead, e.g. pass (D*)0.

This would be letting the implementation drive the interface. Not a good idea in general, unless the interface is inherently inefficient. It isn't.

Daniel Frey:

I agree that the implementation should not drive the interface, but I still fail to see why passing the deleter should be needed anyway. It's not even used in the current enable_shared_from_this implementation, so why do you think that it should be part of sp_accept_owner's interface?

One of your earlier posts contained an example "observer" called shared_ptr_debugger. You didn't show it doing anything, but I can easily imagine it either logging all sp_accept_owner calls, in which case typeid(D).name() could come in handy, or checking whether sp_accept_owner is called twice on the same object. In the latter case, you do want to distinguish between deleterless and deleterful calls, because the same object can legitimately be owned by several shared_ptr instances with null (or effectively null) deleters. The change to the esft base demonstrates another use case, one in which the object wishes to recognize a particular deleter and do something different in sp_accept_owner. It's true that in this particular instance there is another way to achieve the desired "do nothing" behavior. I could however imagine scenarios in which sp_accept_owner and a deleter are used as a pair to achieve postconstructor and predestructor behavior. If the postconstructor action can fail, sp_accept_owner could indicate this by not setting the appropriate "perform predestructor action" flag inside the deleter. We'll see.

On Sun, 2008-04-13 at 05:22 +0300, Peter Dimov wrote:

Daniel Frey:

...

I agree that the implementation should not drive the interface, but I still fail to see why passing the deleter should be needed anyway. It's not even used in the current enable_shared_from_this implementation, so why do you think that it should be part of sp_accept_owner's interface?

One of your earlier posts contained an example "observer" called shared_ptr_debugger. You didn't show it doing anything, but I can easily imagine it either logging all sp_accept_owner calls, in which case typeid(D).name() could come in handy, or checking whether sp_accept_owner is called twice on the same object. In the latter case, you do want to distinguish between deleterless and deleterful calls, because the same object can legitimately be owned by several shared_ptr instances with null (or effectively null) deleters.

The change to the esft base demonstrates another use case, one in which the object wishes to recognize a particular deleter and do something different in sp_accept_owner. It's true that in this particular instance there is another way to achieve the desired "do nothing" behavior.

I could however imagine scenarios in which sp_accept_owner and a deleter are used as a pair to achieve postconstructor and predestructor behavior. If the postconstructor action can fail, sp_accept_owner could indicate this by not setting the appropriate "perform predestructor action" flag inside the deleter.

I think that get_deleter() allows you to implement most of the above features anyway. And I can also imagine to add a more general helper: template< class T > const std::type_info* get_type_info(shared_ptr<T> const& p); which is implemented via a virtual function from sp_counted_base (similar to get_deleter()). The implementation is constructed with knowing the deleter's type, so it could simply return typeid(D). That way even logging typeid(D).name() is possible. This helper is more general as it can be used on any shared_ptr anytime. Regards, Daniel