steinwurf / recycle

Simple resource pool for recycling resources in C++
BSD 3-Clause "New" or "Revised" License
63 stars 23 forks source link

Control block still allocated? #8

Closed SouthernLlama closed 9 years ago

SouthernLlama commented 9 years ago

Hey,

Nice code. But maybe I'm not seeing something correctly. In the allocate() function of the resource pool, every allocate call is creating a new shared pointer from the naked pointer to the managed object. Because you are using a shared pointer constructor to do this, doesn't that mean the control block for the shared pointer is allocated on the heap each call whether it is pooled or not?

Would not that usage of heap allocation every call to allocate() go against the design pattern of a recycler? (Although I give you it is a tiny amount of memory).

mortenvp commented 9 years ago

Sorry for the slow response. I think you are right - I did not see this before. That is a tricky one to remove - any ideas :) ?

SouthernLlama commented 9 years ago

Indeed. As far as I know neither make_shared nor allocate_shared allow for custom deleters to be used. I believe this is only possible through the normal constructor.

Possibly you could maintain a pool of shared_ptr objects and use reset() to assign them on allocation?

SouthernLlama commented 9 years ago

After a fair bit of experimentation and reading the fine print, I have concluded that there is zero capacity for pooling or reusing shared_ptr control blocks directly that is allowed for by the specification. The only legitimate option I have come across for stopping the allocation of a control block when serving recyclable objects is to construct the shared_ptr's using a custom memory allocator that pools the memory of deallocated control blocks for re-use when allocating new control blocks. So for example your allocation function would end like this:

return value_ptr(resource.get(), deleter(pool, resource), poolAllocator);

And then store an instance poolAllocator of the custom allocator in the impl class. Below is a custom allocator implementation I used to do this which is suitable for my needs. Undoubtedly it could be 'more correct', but it seems to work fine on std::shared_ptr<T>.

#pragma once

#include <concurrent_queue.h>

// STL allocator version
template <class T>
class BasicPoolAllocator
{
public:
    typedef T value_type;
    typedef value_type* pointer;
    typedef const value_type* const_pointer;
    typedef value_type& reference;
    typedef const value_type& const_reference;
    typedef typename std::size_t size_type;
    typedef std::ptrdiff_t difference_type;

    template <class tTarget> struct rebind
    {
        typedef BasicPoolAllocator<tTarget> other;
    };

    BasicPoolAllocator() {}
    ~BasicPoolAllocator() {}
    template <class T2> BasicPoolAllocator(const BasicPoolAllocator<T2> &) {}

    pointer address(reference ref)              { return &ref; }
    const_pointer address(const_reference ref)  { return &ref; }

    pointer allocate(size_type n = 1, const void* = 0)
    {
        if (n != 1)
            throw std::bad_alloc(); // Allocator only supports allocations requests of size T.

        pointer place;

        // Try to return pool memory
        bool bOk = freeQueue.try_pop(place);
        if (bOk)
            return place;

        // Generate new pool item
        place = reinterpret_cast<pointer>(operator new(sizeof(value_type)));
        return place;
    }

    void deallocate(pointer ptr, size_type)
    {
        // Note: allocation of size n=1 ==> de-allocation of size n=1
        freeQueue.push(ptr);
    }

    size_type max_size() const
    {
        return 0xffffffffUL / sizeof(value_type);
    }

    void construct(pointer ptr, const value_type & t)
    {
        new(ptr) value_type(t);
    }

    void destroy(pointer ptr)
    {
        ptr->~T();
    }

    void clear_memory()
    {
        pointer place;
        while (freeQueue.try_pop(place))
        {
            ::operator delete(reinterpret_cast<void *>(place));
        }
    }

    template <class T2> bool operator==(BasicPoolAllocator<T2> const&) const
    {
        return true;
    }

    template <class T2> bool operator!=(BasicPoolAllocator<T2> const&) const
    {
        return false;
    }

private:
    //stack to hold pointers to free chunks:
    static concurrency::concurrent_queue<pointer> freeQueue;
};

template<typename T>
concurrency::concurrent_queue<T *> BasicPoolAllocator<T>::freeQueue;
mortenvp commented 9 years ago

Hi @SouthernLlama thanks for the comment. I was thinking about a different approach, using this: https://www.justsoftwaresolutions.co.uk/cplusplus/shared-ptr-secret-constructor.html

... Writing down my idea - I came to the conclusion it would not work :) Anyways I will leave the comment here for future reference.

f18m commented 5 years ago

Hi @SouthernLlama, @mortenvp , @jpihl

I'm commenting on this very old issue that I found very interesting: I'm looking exactly for a way to have zero-malloc reference counted object memory pools in C++.

My question on this issue is: anybody did consider writing a memory pool like the one hosted in this project for a boost::intrusive_ptr ? https://www.boost.org/doc/libs/1_69_0/libs/smart_ptr/doc/html/smart_ptr.html#intrusive_ptr The advantage of boost::intrusive_ptr is that the refcount is held into the allocated object itself. Together with the usage of Boost intrusive lists: https://www.boost.org/doc/libs/1_69_0/doc/html/intrusive/list.html I think that it should be possible to write a template class which is a memory pool for objects T that is

I wonder if such templatized-class would be a suitable PR for this project...

mortenvp commented 5 years ago

@f18m That is an interesting idea. Do you think it is possible to write a generic pool compatible with most intrusive_ptr implementations. Or would this be tied to e.g. boost's variant. I think there is also talk about an intrusive ptr in the standard library.

One additional comment I think the unique_pool also provides a malloc free implementation. But of course if you need shared ownership this is not an option.

f18m commented 5 years ago

Hi @mortenvp , Indeed I needed such a malloc-free memory pool using boost::intrusive_ptr<> so I created this project (a little bit still work in progress): https://github.com/f18m/boost-intrusive-pool

Many ideas come from this recycle project (and another memory pool implementation: https://thinkingeek.com/2017/11/19/simple-memory-pool/), and I put credits for it!!

If you have some time, check it out and let me know what you think!