Maybe the methods of RefCountedObject should be virtual?

[thefreeman]

If this has been mentioned or asked on the forum before I apologize for bringing it up again; I searched for a similar post but couldn't find one.

Is it possible to make the public methods in the RefCountedObject class virtual? I am talking about duplicate(), release() and referenceCount(). If I need/want to use my own implementation of RefCountedObject with a class that inherits from something like Notification I have a problem since NotificationCenter uses Notification instances. So even if I override the duplicate() and release() functions in my classes that extend Notification, these versions won't be called in NotificationCenter unless the methods in RefCountedObject are virtual.

Or am I being silly and there is a better way to accomplish this?

[alex]

> Or am I being silly and there is a better way to accomplish this?

Turning those functions virtual is probably the least disruptive way to immediately achieve what you want. However, I would be hesitant to make them virtual due to performance penalty associated with it.

I don't know enough about your application and what you are trying to do to give a definite answer on this, but if it is generic enough, then making the ))RefCountedObject(( (and/or all classes inheriting from it) policy-based in a similar way we have recently done with ))SharedPtr(( may make sense. Obviously all this should be done with reasonable defaults that do not break existing client code.

At any rate, the core development team is currently not able to do any work regarding this issue. If you are willing to put together a coherent contribution along the above lines, please feel free to do so. The best starting point is to file a [url=http://sourceforge.net/tracker/?group_id=132964&atid=725712|feature request]. If you want SVN write access to publish your work through the http://poco.svn.sourceforge.net/viewvc/poco/sandbox/]sandbox[/url, we'll provide that as well.

[thefreeman]

^> Or am I being silly and there is a better way to accomplish this?

Turning those functions virtual is probably the least disruptive way to immediately achieve what you want. However, I would be hesitant to make them virtual due to performance penalty associated with it. ^

Yes the performance issue is also what bothered me about making the functions virtual.

I don't know enough about your application and what you are trying to do to give a definite answer on this, but if it is generic enough, then making the RefCountedObject (and/or all classes inheriting from it) policy-based in a similar way we have recently done with SharedPtr may make sense. Obviously all this should be done with reasonable defaults that do not break existing client code.

Basically in my case I need the RefCountedObject to not call delete on the object if the reference count reaches zero, but return it to a pool of reusable objects. With regards to making RefCountedObject policy based, are you referring to making it a templated class to allow different implementations of counters? (I am not familiar with the recent changes to SharedPtr). This could work. In the case of the Notification class, one would have to make the Notification class templated as well, with a suitable default template parameter, so that someone that inherits from Notification can change the reference counter implementation.

I would be willing to make these changes.

[alex]

Yes the performance issue is also what bothered me about making the functions virtual.

After thinking a bit about it, for any change, the additional redirection performance penalty is inevitable, but I would still rather pull the counter out and turn the current RefCounterObject into template than make duplicate/release virtual.

Basically in my case I need the RefCountedObject to not call delete on the object if the reference count reaches zero, but return it to a pool of reusable objects. With regards to making RefCountedObject policy based, are you referring to making it a templated class to allow different implementations of counters? (I am not familiar with the recent changes to SharedPtr). This could work. In the case of the Notification class, one would have to make the Notification class templated as well, with a suitable default template parameter, so that someone that inherits from Notification can change the reference counter implementation.

Object pool seems like a good thing to have.

I would be willing to make these changes.

As far as making changes, you can email me your SF username and I'll give you write access to SVN, check the [url=http://pocoproject.org/poco/info/contribute.html|Contributing Mini-FAQ] and http://www.appinf.com/download/CppCodingStyleGuide.pdf]Coding Style Guide[/url. You can create your own sandbox copy of trunk and play there. As for whether your changes will make it back to the trunk, we'll have to wait for Guenter's opinion on this and, of course, check the code.

[thefreeman]

I realized today that making RefCountedObject templated would potentially force all users of Poco to make changes to their code using RefCountedObject. Even with default template parameters, one still has to use RefCountedObject~60~~62~ and not just RefCountedObject to indicate that it is now a templated class. So any classes that users' have written that inherit from RefCountedObject would need to be changed.

The same goes for Notification. In order to allow Notification to use different counters, it also has to be made templated so that a template parameter can be specified for RefCountedObject through Notification. This means that all classes in Poco that use Notification are affected. Some of the functions in NotificationCenter and AbstractObserver would have to be made templated, although this is not the most disruptive issue. NotificationQueue would need to become a templated class so that the correct template specification for Notification can be used in the queue of Notifications that is stored by NotificationQueue. This would again cause problems in existing user code. Also NotificationQueue won't be able to store Notifications with different ReferenceCounter implementations, which could also be a problem.

I am now starting wonder if the advantages of making RefCountedObject templated is worth the disruptions that it would cause?

If one were to modify RefCountedObject in such a way that the release and duplicate functions contain the same code they do now (and remain non-virtual), and just check whether a custom counter has been specified. If no custom counter has been specified the functions continue as normal, if a counter has been specified it calls the release or duplicate metods from the counter. It would then be unnecessary for the release and duplicate functions in RefCountedObject to virtual. A ReferenceCounterBase class with virtual functions can be created, and such an instance could be passed to the constructor of RefCountedObject. This way virtual function and redirection overhead will only come into play if a custom counter is used, and not for the default implementation and no existing implementations using Poco will be affected.

[guenter]

I am a bit reluctant making RefCountedObject template/policy-based, as this will require major changes, including pulling a lot of code into header files, which I'd like to avoid.

What you'd like to achieve can be done in a way simpler way:

Change RefCountedObject in the following way:

1. add a protected virtual member function

Code: Select all


void RefCountedObject::dispose()
{
    delete this;
}


which, in the default implementation, simply calls delete this.

2. change the release() member function so that it calls dispose() instead of delete this.

3. now you can override dispose() and do whatever you like in your subclass, including doing fancy object pooling.

[alex]

I realized today that making RefCountedObject templated would potentially force all users of Poco to make changes to their code using RefCountedObject. Even with default template parameters, one still has to use RefCountedObject~60~~62~ and not just RefCountedObject to indicate that it is now a templated class. So any classes that users' have written that inherit from RefCountedObject would need to be changed.

Not necessarily. RefCountedObject can be renamed and a typedef for that name introduced. That way, no code gets broken. Unrelated to your request, templating the RefCountedObject could be a valid course of action if we want to give an opportunity for avoiding locking that is now imposed. Something similar has been recently done for the SharedPtr.

I am now starting wonder if the advantages of making RefCountedObject templated is worth the disruptions that it would cause?

Well, that is the question. Since I have no time for proper impact analysis, I'll leave it up to you to decide whether you want to pursue this and propose a change (which may not be accepted).

If one were to modify RefCountedObject in such a way that the release and duplicate functions contain the same code they do now (and remain non-virtual), and just check whether a custom counter has been specified. If no custom counter has been specified the functions continue as normal, if a counter has been specified it calls the release or duplicate metods from the counter. It would then be unnecessary for the release and duplicate functions in RefCountedObject to virtual. A ReferenceCounterBase class with virtual functions can be created, and such an instance could be passed to the constructor of RefCountedObject. This way virtual function and redirection overhead will only come into play if a custom counter is used, and not for the default implementation and no existing implementations using Poco will be affected.

I'd rather rename RefCountedObject, make it a template, pull the counter out of it and do the typedef for name RefCountedObject. To try to avoid redirection, duplicate() and release() in a counter can be inlined.

[alex]

I am a bit reluctant making RefCountedObject template/policy-based, as this will require major changes, including pulling a lot of code into header files, which I'd like to avoid.

Not necessarily. While it's true that all current RefCountedObject code must go in header, that's not so much code. Non-breaking change can be done through typedef approach (see my other response). The benefit, on the other side, is that we can provide an opportunity for alternative (e.g. lock-free) approaches like we recently did with SharedPtr.

^
> What you'd like to achieve can be done in a way simpler way:

> Change RefCountedObject in the following way:

> 1. add a protected virtual member function
> which, in the default implementation, simply calls delete this.
^
Just out of curiosity: Provided one uses RefCountedObject (and not a subclass), should the virtual dispose() be inlined and not suffer the runtime redirection penalty due to it's "virtualness"?

[guenter]

> I am a bit reluctant making RefCountedObject template/policy-based, as this will require major changes, including pulling a lot of code into header files, which I'd like to avoid.
>
> Not necessarily. While it's true that all current RefCountedObject code must go in header, that's not so much code. Non-breaking change can be done through typedef approach (see my other response). The benefit, on the other side, is that we can provide an opportunity for alternative (e.g. lock-free) approaches like we recently did with SharedPtr.

But this does not save the problem if someone wants to change refcounting behavior for a subclass of Notification, which is a subclass of RefCountedObject. The only way here is through a virtual function. OTOH, making RefCountedObject policy based is probably a bit overshooting, because if you're using AutoPtr, AutoPtr does not require a RefCountedObject. It only needs an object with duplicate() and release() member functions.

>
> ^
> > What you'd like to achieve can be done in a way simpler way:
>
> > Change RefCountedObject in the following way:
>
> > 1. add a protected virtual member function
> > which, in the default implementation, simply calls delete this.
> ^
> Just out of curiosity: Provided one uses RefCountedObject (and not a subclass), should the virtual dispose() be inlined and not suffer the runtime redirection penalty due to it's "virtualness"?

In most cases, no. Only if the compiler can be really sure that there won't ever be a subclass of the class you're using. But the virtual call here does not cost that much, because it's only done once, before the object is destroyed. I would consider this premature optimization ;-)