1

I'm trying to create a class that keeps a cache of objects. The constructor will be private. Instances will be created with a static function which returns shared_ptrs. I want to use a deleter to remove the instance from the cache (a map) and it seems like I should be able to use a lambda but I cannot get it to work.

Here is what I have. I commented out the line that creates the lambda:

#include <iostream>
#include <cstdlib>
#include <map>
#include <memory>


class CachedThing
{
    public:
        CachedThing(int size) {m_size=size;}
        int m_size;
        static std::map<int, std::shared_ptr<CachedThing>>  m_cache;

    public:
    static void DeleteFromCache(int size)
    {
    }

    static std::shared_ptr<CachedThing> CreateFromCache(int size)
    {
        auto it = CachedThing::m_cache.find(size);
        if (it != CachedThing::m_cache.end())
        {
            std::cout << "FOUND  " << size << std::endl;
            return it->second;
        }

        std::cout << "CREATE " << size << std::endl;

        //auto CacheDeleter = [] (int size) {CachedThing::DeleteFromCache(size);};

        auto f = std::make_shared<CachedThing>(size);  
        m_cache[size]=f;
        return f;
    }
};

std::map<int, std::shared_ptr<CachedThing>> CachedThing::m_cache {};

int main()
{
    std::cout << "Here we go..." << std::endl;

    {
        auto f1 = CachedThing::CreateFromCache(18);
    }
    auto f3 = CachedThing::CreateFromCache(20);
    auto f2 = CachedThing::CreateFromCache(18);
    auto f4 = CachedThing::CreateFromCache(20);
    auto f5 = CachedThing::CreateFromCache(20);

}
Jimbo
  • 85
  • 7
  • 2
    You cannot use `make_shared` to pass a custom deleter, look here: https://stackoverflow.com/questions/34243367/how-to-pass-deleter-to-make-shared – MadScientist Apr 28 '20 at 06:13
  • I tried this and got "error: class template argument deduction failed" auto f = std::shared_ptr(new DXFont(size), [] (int size) {CachedThing::DeleteFromCache(size);}); – Jimbo Apr 28 '20 at 06:26

2 Answers2

4

That wouldn't work. The deleter of the shared_ptr is called once the last instance of the smart pointer is destroyed. In your case you store a copy of the shared_ptr in the cache: that means that you always have at least one instance, and the deleter is never called.

You may employ weak_ptr. This smart pointer has the access to the same counter that the shared_ptr uses to count instances, but weak_ptr has no ownership and doesn't increase/decrease the counter. You may create another shared_ptr instance out of a weak_ptr whenever at least one shared_ptr pointing on the object exists.

So the cache would store weak_ptrs, you may remove this weak_ptr from the deleter.

Here is the updated version that uses weak_ptrs:

#include <iostream>
#include <cstdlib>
#include <map>
#include <memory>


class CachedThing
{
    public:
        CachedThing(int size) {m_size=size;}
        int m_size;
        static std::map<int, std::weak_ptr<CachedThing>>  m_cache;

    public:
    static void DeleteFromCache(int size)
    {
    }

    static std::shared_ptr<CachedThing> CreateFromCache(int size)
    {
        auto it = CachedThing::m_cache.find(size);
        if (it != CachedThing::m_cache.end())
        {
            std::cout << "FOUND  " << size << std::endl;
            return it->second.lock();
        }

        std::cout << "CREATE " << size << std::endl;

        auto CacheDeleter = [] (CachedThing *obj) {
            std::cout << "Deleter called for " << obj->m_size << std::endl;
            CachedThing::DeleteFromCache(obj->m_size);
            delete obj;
        };

        std::shared_ptr<CachedThing> f(new CachedThing(size), CacheDeleter);  
        m_cache[size]=f;
        return f;
    }
};

std::map<int, std::weak_ptr<CachedThing>> CachedThing::m_cache {};

int main()
{
    std::cout << "Here we go..." << std::endl;

    {
        auto f1 = CachedThing::CreateFromCache(18);
    }
    auto f3 = CachedThing::CreateFromCache(20);
    auto f2 = CachedThing::CreateFromCache(18);
    auto f4 = CachedThing::CreateFromCache(20);
    auto f5 = CachedThing::CreateFromCache(20);

}
Dmitry Kuzminov
  • 6,180
  • 6
  • 18
  • 40
  • Ah ok I thought the deleter might be called whenever the shared_ptr use count goes down, but it sounds like you are saying it only gets called when the last instance is destroyed. So using weak_ptrs I would periodically (on every get maybe) check the map for any weak_ptrs that are expired and remove them from the map. Makes sense. – Jimbo Apr 28 '20 at 06:30
  • 1
    @Jimbo, no, you still can use this approach and remove an expired `weak_ptr` from the deleter (no need in periodical garbage collector). However the reason of having cache is not obvious from your code. What is the policy of keeping/removing the object from cache? – Dmitry Kuzminov Apr 28 '20 at 06:33
  • The intent is: The first time an instance is needed for a given set of arguments it is instantiated, added to a cache, and a pointer to the instance is returned. On subsequent requests for an instance matching the arguments, pointers to the cached object will be returned. Instances will be stored in the cache as long as the returned pointers are in use. There is no limit to the size of the cache. – Jimbo Apr 28 '20 at 06:44
  • @Jimbo, Ok, then my implementation shall solve your problem. – Dmitry Kuzminov Apr 28 '20 at 06:45
0

My final solution, based on Dmitry's answer, is:

#include <iostream>
#include <cstdlib>
#include <map>
#include <memory>


class CachedThing
{
    private:
        CachedThing() = delete;
        CachedThing(int size) {m_size=size;}
        int m_size;
        static std::map<int, std::weak_ptr<CachedThing>>  m_cache;

    public:

    static std::shared_ptr<CachedThing> CreateFromCache(int size)
    {
        auto it = CachedThing::m_cache.find(size);
        if (it != CachedThing::m_cache.end())
        {
            std::cout << "FOUND  " << size << std::endl;
            return it->second.lock();
        }

        std::cout << "CREATE " << size << std::endl;

        auto CacheDeleter = [] (CachedThing *obj) {
            auto it=m_cache.find (obj->m_size);
            m_cache.erase ( it, m_cache.end() );  
            std::cout << "DELETE " << obj->m_size << std::endl;
            delete obj;
        };

        std::shared_ptr<CachedThing> f(new CachedThing(size), CacheDeleter);  
        m_cache[size]=f;
        return f;
    }
};

std::map<int, std::weak_ptr<CachedThing>> CachedThing::m_cache {};

int main()
{
    std::cout << "Here we go..." << std::endl;

    {
        auto f1 = CachedThing::CreateFromCache(18);
    }
    auto f3 = CachedThing::CreateFromCache(20);
    auto f2 = CachedThing::CreateFromCache(18);
    auto f4 = CachedThing::CreateFromCache(20);
    auto f5 = CachedThing::CreateFromCache(20);

}
Jimbo
  • 85
  • 7