Как я могу уменьшить количество вызовов конструкторов в моих двух классах?

#c #oop #constructor

#c #ооп #конструктор

Вопрос:

Я пишу Cache класс, который действует как расширение std::unordered_map , которое подсчитывает ссылки и автоматически удаляет записи, на которые нет ссылок. Он также имеет логический параметр, позволяющий разработчику выбирать, хочет ли он сохранить запись на карте, даже если ссылок нет. Этот класс возвращает значения в виде CacheRef экземпляра. Вот определения для указанных классов:

 #pragma once

#include <iostream>
#include <unordered_map>
#include <tuple>
#include <string>
#include <utility>

template <typename TKey, typename TValue>
class Cache;

template <typename TKey, typename TValue>
class CacheRef
{
private:
    using CacheMap = std::unordered_map<TKey, std::tuple<TValue, int, bool>>;
public:
    TValue* value = nullptr;
    
    CacheRef<TKey, TValue>amp; operator=(const CacheRef<TKey, TValue>amp;) = delete;
    CacheRef<TKey, TValue>amp; operator=(CacheRef<TKey, TValue>amp;amp; other) = delete;
    CacheRef(const CacheRef<TKey, TValue>amp; ref) :
    m_validRef(ref.m_validRef), m_valueLocation(ref.m_valueLocation), m_cache(ref.m_cache)
    {
        std::cout << "Reference constructor called" << std::endl;
        initialize();
    }

    CacheRef(CacheRef<TKey, TValue>amp;amp; ref) :
    m_validRef(ref.m_validRef), m_valueLocation(ref.m_valueLocation), m_cache(ref.m_cache)
    {
        std::cout << "Move constructor called" << std::endl;
        initialize();
    }

    CacheRef(typename CacheMap::iterator valueLocation, Cache<TKey, TValue>* cache, bool validRef = true)
        : m_valueLocation(valueLocation), m_cache(cache), m_validRef(validRef)
    {
        std::cout << "Normal constructor called" << std::endl;
        initialize();
    }

    ~CacheRef()
    {
        std::cout << "Destructor called" << std::endl;
        if (m_validRef)
        {
            m_cache->deleteReference(m_valueLocation);
        }
    }

private:
    void initialize()
    {
        if (m_validRef)
        {
            value = amp;std::get<0>(m_valueLocation->second);
              std::get<1>(m_valueLocation->second);
        }
    }
    typename CacheMap::iterator m_valueLocation;
    const bool m_validRef;
    Cache<TKey, TValue>* const m_cache;
};

template <typename TKey, typename TValue>
class Cache
{
    using ValueInfo = std::tuple<TValue, int, bool>;
    using CacheMap = std::unordered_map<TKey, ValueInfo>;
public:
    Cache() = default;
    ~Cache() = default;
    CacheRef<TKey, TValue> add(const TKeyamp; key, const TValueamp; value, bool autoCacheDelete = true)
    {
        // Make sure TValue is not in the cache already
        auto pair = find(key);
        if (pair.second)
        {
            return pair.first;
        }
        else
        {
            // Insert in cache map
            ValueInfo val(value, 0, autoCacheDelete);
            std::pair<typename CacheMap::iterator, bool> insert = m_cache.insert(std::make_pair(key, val));
                
            // Return the newly inserted element as a CacheRef
            return CacheRef<TKey, TValue>(insert.first, this, true);
        }
    }
    
    std::pair<CacheRef<TKey, TValue>, bool> find(const TKeyamp; key)
    {
        typename CacheMap::iterator kvp = m_cache.find(key);
        if (kvp == m_cache.end())
        {
            return std::pair<CacheRef<TKey, TValue>, bool>(CacheRef<TKey, TValue>(kvp, this, false), false);
        }
        else
        {
            return std::pair<CacheRef<TKey, TValue>, bool>(CacheRef<TKey, TValue>(kvp, this, true), true);
        }
    }

    void print()
    {
        for (auto element : m_cache)
        {
            std::cout << "Key: " << element.first << "tValue:" << std::get<0>(element.second) << " #Ref: " << std::get<1>(element.second) << std::endl;
        }

        std::cout << std::endl;
    }
        
    void deleteReference(typename CacheMap::iterator where)
    {
        intamp; numRef = std::get<1>(where->second);
        --numRef;
        if (numRef == 0 amp;amp; std::get<2>(where->second))
        {
            m_cache.erase(where);
        }
    }

private:
    CacheMap m_cache;
};

  

Вот тестовая функция, которая выполняется

 int main()
{
    Cache<std::string, int> testCache;
    auto ref = testCache.add("myKey", 10);
    testCache.print();
}
  

И вот результат

 > Normal constructor called //<-- This is an invalid reference being created by Cache::find() called by Cache::add() to check if data already exists
> Move constructor called //<-- Probably called by std::pair constructor from Cache::find()
> Destructor called //<-- First data being destroyed
> Normal constructor called //<-- Real data is created by Cache::add()
> Destructor called //<-- Destroyed the return value of Cache::find() when leaving Cache::add() scope
> Key: myKey    Value:10 #Ref: 1
  

Итак, я думаю, есть некоторые очевидные оптимизации, которые можно выполнить. Первое, о чем я думаю, это удалить вызов find() in add() , но тогда это потребует от меня большей осторожности в отношении того, как я использую класс, поэтому я бы предпочел избежать этого.

Кроме того, создание недопустимой ссылки требует двух вызовов конструктора только для того, чтобы выбросить экземпляр, который я не считаю очень чистым. На самом деле вся идея недопустимой ссылки меня немного беспокоит, но я не мог придумать никакого другого возвращаемого значения, когда find() не могу найти совпадение

Комментарии:

1. Какие параметры компилятора вы использовали для создания своего приложения? Включена ли у вас оптимизация в ваших командах компиляции сборки?

2. Одним из способов было add бы не вызывать ваш find , а вызывать m_cache.find(key); напрямую.

Ответ №1:

Старый вопрос, который я копаю. Я бы использовал std::optional в качестве возвращаемого типа from find . Таким образом, вы не создаете временные объекты:

 #include <iostream>
#include <unordered_map>
#include <optional>

template <typename TValue>
struct ValueInfo {
    TValue value;
    int refCount;
    bool autoCacheDelete;
};

template <typename TKey, typename TValue>
class Cache;

template <typename TKey, typename TValue>
class CacheRef
{
private:
    using CacheMap = std::unordered_map<TKey, ValueInfo<TValue>>;
    using CacheMapIterator = CacheMap::iterator;
    TValue* value = nullptr;
public:
    CacheRefamp; operator=(CacheRef constamp;) = delete;
    CacheRefamp; operator=(CacheRefamp;amp;) = delete;

    CacheRef(CacheRef constamp; ref)
    : _validRef(ref._validRef), _valueLocation(ref._valueLocation), _cache(ref._cache)
    {
        std::cout << "Reference constructor calledn";
        initialize();
    }

    CacheRef(CacheRefamp;amp; ref)
    : _validRef(ref._validRef), _valueLocation(ref._valueLocation), _cache(ref._cache)
    {
        std::cout << "Move constructor calledn";
        initialize();
    }

    CacheRef(CacheMapIterator valueLocation, Cache<TKey, TValue>* cache, bool validRef = true)
    : _valueLocation(valueLocation), _cache(cache), _validRef(validRef)
    {
        std::cout << "Normal constructor calledn";
        initialize();
    }

    ~CacheRef()
    {
        std::cout << "Destructor calledn";
        if (_validRef)
        {
            _cache->deleteReference(_valueLocation);
        }
    }

private:
    void initialize()
    {
        if (_validRef)
        {
            value = amp;_valueLocation->second.value;
              _valueLocation->second.refCount;
        }
    }
    CacheMapIterator _valueLocation;
    const bool _validRef;
    Cache<TKey, TValue>* const _cache;
};

template <typename TKey, typename TValue>
class Cache
{
private:
    using CacheMap = std::unordered_map<TKey, ValueInfo<TValue>>;
    using CacheMapIterator = CacheMap::iterator;
public:
    Cache() = default;

    CacheRef<TKey, TValue> add(const TKeyamp; key, const TValueamp; value, bool autoCacheDelete = true)
    {
        // Make sure TValue is not in the cache already
        auto cacheRef = find(key);
        if (cacheRef.has_value()) return cacheRef.value();
        else {
            // Insert in cache map
            ValueInfo val(value, 0, autoCacheDelete);
            auto [iterator, succeeded] = _cache.insert(std::make_pair(key, val));
            // should prolly check 'succeeded'
            // Return the newly inserted element as a CacheRef
            return CacheRef<TKey, TValue>(iterator, this, true);
        }
    }
    
    std::optional<CacheRef<TKey, TValue>> find(const TKeyamp; key)
    {
        auto cacheIt = _cache.find(key);
        if (cacheIt == _cache.end()) return std::nullopt;
        return CacheRef<TKey, TValue>(cacheIt, this, true);
    }

    void print()
    {
        for (auto constamp; [key, cacheRef] : _cache)
        {
            std::cout << "Key: " << key << "tValue:" << cacheRef.value << " #Ref: " << cacheRef.refCount << 'n';
        }
    }
        
    void deleteReference(CacheMapIterator where)
    {
        intamp; numRef = where->second.refCount;
        --numRef;
        if (numRef == 0 amp;amp; where->second.autoCacheDelete)
        {
            _cache.erase(where);
        }
    }

private:
    CacheMap _cache;
};

int main()
{
    Cache<std::string, int> testCache;
    auto ref = testCache.add("myKey", 10);
    testCache.print();
}
  

дает:

 Normal constructor called
Key: myKey  Value:10 #Ref: 1
Destructor called