[smart_ptr] Proposal to extract intrusive_ref_counter from Boost.Log
Hi, I'd like to move intrusive_ref_counter class from Boost.Log to Boost.SmartPtr. The class is intended to be used as a base class for a user's class that is intended to be used with intrusive_ptr. The class implements a reference counter and the related intrusive_ptr_add_ref/intrusive_ptr_release operations, so the user doesn't have to do that himself. Usage example: class my_class : public intrusive_ref_counter { }; intrusive_ptr< my_class > p = new my_class(); Besides just simplifying intrusive_ptr usage, the base class can be used as a sort of "void" as in shared_ptr< void >. I.e. intrusive_ptr< intrusive_ref_counter > can refer to object of any class that follows this protocol. Implementation: boost/log/utility/intrusive_ref_counter.hpp
hey andrey,
I'd like to move intrusive_ref_counter class from Boost.Log to Boost.SmartPtr. The class is intended to be used as a base class for a user's class that is intended to be used with intrusive_ptr. The class implements a reference counter and the related intrusive_ptr_add_ref/intrusive_ptr_release operations, so the user doesn't have to do that himself.
if it saves some error-prone boilerplate code, it is very useful imo! cheers, tim
Andrey Semashev
I'd like to move intrusive_ref_counter class from Boost.Log to Boost.SmartPtr.
This seems like a good idea. However, it seems that you would want to separate it into two separate classes, one that uses atomic operations and one that doesn't, rather than having the implementation be controlled by preprocessor definitions.
On Wednesday 28 August 2013 22:44:54 Jeremy Maitin-Shepard wrote:
Andrey Semashev
writes: I'd like to move intrusive_ref_counter class from Boost.Log to Boost.SmartPtr.
This seems like a good idea. However, it seems that you would want to separate it into two separate classes, one that uses atomic operations and one that doesn't, rather than having the implementation be controlled by preprocessor definitions.
The config macro is related to Boost.Log. I was thinking about changing it to BOOST_NO_THREADS after the extraction, so that the counter is always atomic in multithreaded environments. But I can make it a template parameter policy, if it's needed.
Andrey Semashev wrote:
The config macro is related to Boost.Log. I was thinking about changing it to BOOST_NO_THREADS after the extraction, so that the counter is always atomic in multithreaded environments.
atomic_count already does this, so there should be no need to replicate the #ifdef at the point of use.
On Friday 30 August 2013 10:52:35 Peter Dimov wrote:
Andrey Semashev wrote:
The config macro is related to Boost.Log. I was thinking about changing it to BOOST_NO_THREADS after the extraction, so that the counter is always atomic in multithreaded environments.
atomic_count already does this, so there should be no need to replicate the #ifdef at the point of use.
Ok, I didn't notice that, thanks. So I take it you don't mind me moving this component to Boost.SmartPtr?
Andrey Semashev wrote:
On Friday 30 August 2013 10:52:35 Peter Dimov wrote: ...
atomic_count already does this, so there should be no need to replicate the #ifdef at the point of use.
Ok, I didn't notice that, thanks.
So I take it you don't mind me moving this component to Boost.SmartPtr?
I don't mind, but it needs a test or two, a documentation page, and the intrusive_ptr documentation probably ought to be updated to reference it.
On Friday 30 August 2013 14:55:07 Peter Dimov wrote:
Andrey Semashev wrote:
On Friday 30 August 2013 10:52:35 Peter Dimov wrote: ...
atomic_count already does this, so there should be no need to replicate the #ifdef at the point of use.
Ok, I didn't notice that, thanks.
So I take it you don't mind me moving this component to Boost.SmartPtr?
I don't mind, but it needs a test or two, a documentation page, and the intrusive_ptr documentation probably ought to be updated to reference it.
On 31.8.2013. 21:55, Andrey Semashev wrote:
On Friday 30 August 2013 14:55:07 Peter Dimov wrote:
Andrey Semashev wrote:
On Friday 30 August 2013 10:52:35 Peter Dimov wrote: So I take it you don't mind me moving this component to Boost.SmartPtr?
I don't mind, but it needs a test or two, a documentation page, and the intrusive_ptr documentation probably ought to be updated to reference it.
Done.
Given that one of the main 'points' of intrusive_ptr is efficiency, forcing a virtual destructor is IMO completely misguided. Policies that would (also) allow CRTP+protected-destructor and custom deleters would make a more complete product. IMO there is no need for a class that applies shared_ptr's "I don't care what it costs" design to intrusive_ptr. The polymorphic version should also mark the destructor as nothrow, otherwise every function that deals with such an intrusive_ptr in a non trivial way (i.e. implicitly calls intrusive_ptr_release(const basic_intrusive_ref_counter< CounterPolicyT >*) is a possibly throwing function for the compiler... No offence but if you use such an approach even with the low level components no wonder Boost.Log is the most bloated Boost library... -- "What Huxley teaches is that in the age of advanced technology, spiritual devastation is more likely to come from an enemy with a smiling face than from one whose countenance exudes suspicion and hate." Neil Postman
On Sunday 01 September 2013 21:13:08 Domagoj Saric wrote:
On 31.8.2013. 21:55, Andrey Semashev wrote:
On Friday 30 August 2013 14:55:07 Peter Dimov wrote:
Andrey Semashev wrote:
On Friday 30 August 2013 10:52:35 Peter Dimov wrote: So I take it you don't mind me moving this component to Boost.SmartPtr?
I don't mind, but it needs a test or two, a documentation page, and the intrusive_ptr documentation probably ought to be updated to reference it.
Done.
Given that one of the main 'points' of intrusive_ptr is efficiency, forcing a virtual destructor is IMO completely misguided. Policies that would (also) allow CRTP+protected-destructor and custom deleters would make a more complete product.
CRTP version might be a good addition. Care to submit?
The polymorphic version should also mark the destructor as nothrow, otherwise every function that deals with such an intrusive_ptr in a non trivial way (i.e. implicitly calls intrusive_ptr_release(const basic_intrusive_ref_counter< CounterPolicyT >*) is a possibly throwing function for the compiler...
Destructors are implicitly noexcept in C++11.
No offence but if you use such an approach even with the low level components no wonder Boost.Log is the most bloated Boost library...
That was a bit harsh, you know. The base class is used in Boost.Log where there are virtual functions and destructor anyway. If you have particular complaints and suggestions about Boost.Log I'd be happy to discuss them in a separate topic.
On Monday 02 September 2013 00:04:15 you wrote:
On Sunday 01 September 2013 21:13:08 Domagoj Saric wrote:
Given that one of the main 'points' of intrusive_ptr is efficiency, forcing a virtual destructor is IMO completely misguided. Policies that would (also) allow CRTP+protected-destructor and custom deleters would make a more complete product.
CRTP version might be a good addition. Care to submit?
Actually, I liked that idea so much that I have changed the implementation to follow that design. Thanks for the suggestion. https://svn.boost.org/trac/boost/changeset/85547
participants (5)
-
Andrey Semashev -
Domagoj Saric -
Jeremy Maitin-Shepard -
Peter Dimov -
Tim Blechmann