3

This is becoming a common pattern in my code, for when I need to manage an object that needs to be noncopyable because either A. it is "heavy" or B. it is an operating system resource, such as a critical section:

class Resource;

class Implementation : public boost::noncopyable
{
    friend class Resource;
    HANDLE someData;
    Implementation(HANDLE input) : someData(input) {};
    void SomeMethodThatActsOnHandle() {
        //Do stuff
    };
public:
    ~Implementation() { FreeHandle(someData) };
};

class Resource
{
    boost::shared_ptr<Implementation> impl;
public:
    Resource(int argA) explicit {
        HANDLE handle = 
            SomeLegacyCApiThatMakesSomething(argA);
        if (handle == INVALID_HANDLE_VALUE)
            throw SomeTypeOfException();
        impl.reset(new Implementation(handle));
    };
    void SomeMethodThatActsOnTheResource() {
        impl->SomeMethodThatActsOnTheHandle();
    };
};

This way, shared_ptr takes care of the reference counting headaches, allowing Resource to be copyable, even though the underlying handle should only be closed once all references to it are destroyed.

However, it seems like we could save the overhead of allocating shared_ptr's reference counts and such separately if we could move that data inside Implementation somehow, like boost's intrusive containers do.

If this is making the premature optimization hackles nag some people, I actually agree that I don't need this for my current project. But I'm curious if it is possible.

skaffman
  • 398,947
  • 96
  • 818
  • 769
Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • Boost has an `intrusive_ptr`: http://www.boost.org/doc/libs/1_42_0/libs/smart_ptr/intrusive_ptr.html But I'm not posting as an answer because I'm not sure if that's what you want. – GManNickG Apr 02 '10 at 20:38
  • 1
    Also, THIS IS TOTALLY PREMATURE OPTIMIZATION WTF ARE YOU DOING? DON'T YOU KNOW KNUTH'S QUOTE? PREMATURE OPTIMIZATION IS THE ROOT OF ALL EVIL? HM, HAVEN'T YOU HEARD IT YET? NOW YOU HAVE. – GManNickG Apr 02 '10 at 20:39
  • 1
    @GMan: LOL! Though @others reading this, I believe the quote is "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil". I'm stipulating this is a 3% case. – Billy ONeal Apr 02 '10 at 20:53
  • I would change your pattern so that the whole of the implementation is in the Implementation class. Once that is done you do not need a separate Resource class to hold the smart pointer. You could use a base class of Implementation to provide a synchronized ref count. The two functions at global scope have to be provided per class and defined in the same namespace as your class so you might want a pair of macros for that, or a template per namespace. – Permaquid Apr 03 '10 at 12:20
  • @Permaquid: That defeats the purpose of putting Implementation by itself in the first place. The reason for the Resource class is the assumption that Implementation must be noncopyable for some reason, for example a CRITICAL_SECTION object. If it was copyable, I could just put the HANDLE into a boost::shared_ptr and be done with it. – Billy ONeal Apr 03 '10 at 19:09
  • To be fair, my example doens't make that super duper clear :( – Billy ONeal Apr 03 '10 at 19:12
  • What about shared_ptr allows the resource to be copied? – Permaquid Apr 07 '10 at 17:33

4 Answers4

6

Use a boost::intrusive_ptr which is designed to work on a class with an embedded reference count.

Non-tested example based on example here:

class Resource; 

class Implementation : public boost::noncopyable 
{ 
    friend class Resource;
    HANDLE someData;
    int refCount; // The reference count.
    Implementation(HANDLE input) : someData(input) { refCount = 0; }; 
    void SomeMethodThatActsOnHandle() { 
        //Do stuff 
    }; 
public: 
    ~Implementation() { FreeHandle(someData) }; 
};

intrusive_ptr_add_ref(Implementation* imp) { imp->refCount++; }
intrusive_ptr_release(Implementation* imp) { if(--imp->refCount) == 0) delete imp; }

class Resource 
{ 
    boost::intrusive_ptr<Implementation> impl; 
public: 
    Resource(int argA) explicit { 
        HANDLE handle =  
            SomeLegacyCApiThatMakesSomething(argA); 
        if (handle == INVALID_HANDLE_VALUE) 
            throw SomeTypeOfException(); 
        impl.reset(new Implementation(handle)); 
    }; 
    void SomeMethodThatActsOnTheResource() { 
        impl->SomeMethodThatActsOnTheHandle(); 
    }; 
}; 
Anders Abel
  • 67,989
  • 17
  • 150
  • 217
  • Can you provide an example (preferably the above one edited) that uses `intrusive_ptr`? I saw that in the docs, but the documentation is a bit opaque... – Billy ONeal Apr 02 '10 at 20:43
  • 2
    Don't forget to make the increment and decrement-and-test atomic. – Permaquid Apr 03 '10 at 11:59
  • @Permaquid: In a multithreaded environment that is needed yes. My code above is *not* intended to be thread safe. – Anders Abel Apr 03 '10 at 12:33
  • @Permaquid: Given that it's not atomic in boost::shared_ptr I don't think it's really an issue here. – Billy ONeal Apr 03 '10 at 19:11
  • 1
    @BillyONeal: Yes it is, mostly as a lock-free implementation. Search for *lock-free* on [this](http://www.boost.org/doc/libs/1_42_0/libs/smart_ptr/shared_ptr.htm) page. – Anders Abel Apr 04 '10 at 06:36
  • The question mentioned critical sections. My comment is just a reminder to the questioner, based on that. @BillyONeal the reference counts are managed using atomic operations in shared_ptr. – Permaquid Apr 07 '10 at 17:28
6

A partial solution is to use make_shared to create your shared_ptrs. For example,

auto my_thing = std::make_shared<Thing>();

instead of

auto my_thing = std::shared_ptr<Thing>(new Thing);

It's still non-intrusive, so nothing else needs to change. Good implementations of make_shared combine the memory allocation for the reference count and the object itself. That saves a memory allocation and keeps the count close to the object for better locality. It's not quite as efficient as something like boost:intrusive_ptr, but it's worth considering.

Adrian McCarthy
  • 45,555
  • 16
  • 123
  • 175
  • I can't do this because the shared pointer is constructed as a member of `Resource` before I have constructed the argument needed for `Implementation`. You end up constructing a shared_ptr to NULL, and then assigning it a new shared_ptr allocated with make_shared. The allocation cost was already paid by by impl's call before the constructor started executing. – Billy ONeal Apr 02 '10 at 22:53
  • 2
    @BillyONeal: A default constructed shared_ptr shouldn't need to allocate anything until reset is called. It should be very cheap. – dvide Apr 03 '10 at 10:04
1

you can save some overhead by simply getting rid of the two classes and having just one and typedefing a shared ptr to it - this is the idom i use all the time

 class Resource
 {
      ...
 };
 typedef boost::shared_ptr<Resource> ResourcePtr;
Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
pm100
  • 48,078
  • 23
  • 82
  • 145
  • That still requires users to choose whether to use the pointer variant or not. I'd rather avoid that. Plus it does not answer the question I asked. – Billy ONeal Apr 02 '10 at 20:41
1

If you want to reduce the overhead of distinct memory allocations for your object and the reference counter, you could try to use make_shared. That's what its for.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
sellibitze
  • 27,611
  • 3
  • 75
  • 95
  • I can't do that because shared_ptr is constructed before I know what to assign to it in my constructor. – Billy ONeal Apr 02 '10 at 20:45
  • @BillyONeal: So? I don't see why make_shared isn't applicable. Creating a shared_ptr that points to nothing doesn't require a ref counter. – sellibitze Apr 03 '10 at 08:36
  • I think this would be just as efficient as using an `intrusive_ptr`. `intrusive_ptr` is good for managing resources that already have a reference count like COM objects. But I don't think there is much to be gained by explicitly coding a reference count in when it's not needed. As is mentioned here, `make_shared` should allocate the reference count and `Implementation` in one go, so that step should be no different than newing up an `Implementation` that contains a reference count inside itself. – dvide Apr 03 '10 at 10:21