Rename size to cost

To fixup between the two first commits.

Author: mgautierfr

src/concurrent_cache.h

@@ -103,10 +103,10 @@ class ConcurrentCache: private lru_cache<Key, std::shared_future<Value>, FutureT
         // should not be the least used item.
         // If it happens, this should not be a problem if current_size is bigger than `cost` (most of the time)
         // For the really rare specific case of current cach size being lower than `cost` (if possible),
-        // `decreaseSize` will clamp the new size to 0.
+        // `decreaseCost` will clamp the new size to 0.
         {
           std::unique_lock<std::mutex> l(lock_);
-          Impl::increaseSize(cost);
+          Impl::increaseCost(cost);
         }
       } catch (std::exception& e) {
         drop(key);
@@ -131,17 +131,17 @@ class ConcurrentCache: private lru_cache<Key, std::shared_future<Value>, FutureT
 
   size_t getMaxSize() const {
     std::unique_lock<std::mutex> l(lock_);
-    return Impl::getMaxSize();
+    return Impl::getMaxCost();
   }
 
   size_t getCurrentSize() const {
     std::unique_lock<std::mutex> l(lock_);
-    return Impl::size();
+    return Impl::cost();
   }
 
   void setMaxSize(size_t newSize) {
     std::unique_lock<std::mutex> l(lock_);
-    return Impl::setMaxSize(newSize);
+    return Impl::setMaxCost(newSize);
   }
 
 private: // data

src/dirent_accessor.h

@@ -55,9 +55,9 @@ class LIBZIM_PRIVATE_API DirectDirentAccessor
   std::shared_ptr<const Dirent> getDirent(entry_index_t idx) const;
   entry_index_t getDirentCount() const  {  return m_direntCount; }
 
-  size_t getMaxCacheSize() const { return m_direntCache.getMaxSize(); }
-  size_t getCurrentCacheSize() const { return m_direntCache.size(); }
-  void setMaxCacheSize(size_t nbDirents) const { m_direntCache.setMaxSize(nbDirents); }
+  size_t getMaxCacheSize() const { return m_direntCache.getMaxCost(); }
+  size_t getCurrentCacheSize() const { return m_direntCache.cost(); }
+  void setMaxCacheSize(size_t nbDirents) const { m_direntCache.setMaxCost(nbDirents); }
 
 private: // functions
   std::shared_ptr<const Dirent> readDirent(offset_t) const;

src/lrucache.h

@@ -71,6 +71,9 @@ struct UnitCostEstimation {
  * This lru is parametrized by a CostEstimation type. The type must have a static method `cost`
  * taking a reference to a `value_t` and returing its "cost". As already said, this method must
  * always return the same cost for the same value.
+ *
+ * While cost could be any kind of value, this implemention is intended to be used only with
+ * `UnitCostEstimation` (classic lru) and `FutureToValueCostEstimation<ClusterMemorySize>`.
  */
 template<typename key_t, typename value_t, typename CostEstimation>
 class lru_cache {
@@ -107,9 +110,9 @@ class lru_cache {
   };
 
 public: // functions
-  explicit lru_cache(size_t max_size) :
-    _max_size(max_size),
-    _current_size(0)
+  explicit lru_cache(size_t max_cost) :
+    _max_cost(max_cost),
+    _current_cost(0)
   {}
 
   // If 'key' is present in the cache, returns the associated value,
@@ -130,8 +133,8 @@ class lru_cache {
     auto it = _cache_items_map.find(key);
     if (it != _cache_items_map.end()) {
       _cache_items_list.splice(_cache_items_list.begin(), _cache_items_list, it->second);
-      decreaseSize(CostEstimation::cost(it->second->second));
-      increaseSize(CostEstimation::cost(value));
+      decreaseCost(CostEstimation::cost(it->second->second));
+      increaseCost(CostEstimation::cost(value));
       it->second->second = value;
     } else {
       putMissing(key, value);
@@ -155,7 +158,7 @@ class lru_cache {
     } catch (std::out_of_range& e) {
       return false;
     }
-    decreaseSize(CostEstimation::cost(list_it->second));
+    decreaseCost(CostEstimation::cost(list_it->second));
     _cache_items_list.erase(list_it);
     _cache_items_map.erase(key);
     return true;
@@ -180,56 +183,56 @@ class lru_cache {
     return _cache_items_map.find(key) != _cache_items_map.end();
   }
 
-  size_t size() const {
-    return _current_size;
+  size_t cost() const {
+    return _current_cost;
   }
 
-  size_t getMaxSize() const {
-    return _max_size;
+  size_t getMaxCost() const {
+    return _max_cost;
   }
 
-  void setMaxSize(size_t newSize) {
-    while (newSize < this->size()) {
+  void setMaxCost(size_t newMaxCost) {
+    while (newMaxCost < this->cost()) {
       dropLast();
     }
-    _max_size = newSize;
+    _max_cost = newMaxCost;
   }
 
  protected:
 
-  void increaseSize(size_t extra_size) {
+  void increaseCost(size_t extra_cost) {
     // increaseSize is called after we have added a value to the cache to update
     // the size of the current cache.
     // We must ensure that we don't drop the value we just added.
     // While it is technically ok to keep no value if max cache size is 0 (or memory size < of the size of one cluster)
     // it will make recreate the value all the time.
     // Let's be nice with our user and be tolerent to misconfiguration.
-    if (!extra_size) {
+    if (!extra_cost) {
       // Don't try to remove an item if we have new size == 0.
       // This is the case when concurent cache add a future without value.
       // We will handle the real increase size when concurent cache will directly call us.
       return;
     }
-    _current_size += extra_size;
-    while (_current_size > _max_size && _cache_items_list.size() > 1) {
+    _current_cost += extra_cost;
+    while (_current_cost > _max_cost && size() > 1) {
       dropLast();
     }
   }
 
-  void decreaseSize(size_t sizeToRemove) {
-    if (sizeToRemove > _current_size) {
-      std::cerr << "WARNING: We have detected inconsistant cache management, trying to remove " << sizeToRemove << " from a cache with size " << _current_size << std::endl;
+  void decreaseCost(size_t costToRemove) {
+    if (costToRemove > _current_cost) {
+      std::cerr << "WARNING: We have detected inconsistant cache management, trying to remove " << costToRemove << " from a cache with size " << _current_cost << std::endl;
       std::cerr << "Please open an issue on https://github.com/openzim/libzim/issues with this message and the zim file you use" << std::endl;
-      _current_size = 0;
+      _current_cost = 0;
     } else {
-      _current_size -= sizeToRemove;
+      _current_cost -= costToRemove;
     }
   }
 
 private: // functions
   void dropLast() {
     auto list_it = _cache_items_list.back();
-    decreaseSize(CostEstimation::cost(list_it.second));
+    decreaseCost(CostEstimation::cost(list_it.second));
     _cache_items_map.erase(_cache_items_list.back().first);
     _cache_items_list.pop_back();
   }
@@ -238,14 +241,19 @@ class lru_cache {
     assert(_cache_items_map.find(key) == _cache_items_map.end());
     _cache_items_list.push_front(key_value_pair_t(key, value));
     _cache_items_map[key] = _cache_items_list.begin();
-    increaseSize(CostEstimation::cost(value));
+    increaseCost(CostEstimation::cost(value));
+  }
+
+  size_t size() const {
+    return _cache_items_map.size();
   }
 
+
 private: // data
   std::list<key_value_pair_t> _cache_items_list;
   std::map<key_t, list_iterator_t> _cache_items_map;
-  size_t _max_size;
-  size_t _current_size;
+  size_t _max_cost;
+  size_t _current_cost;
 };
 
 } // namespace zim

test/lrucache.cpp

@@ -42,7 +42,7 @@ TEST(CacheTest, SimplePut) {
     cache_lru.put(7, 777);
     EXPECT_TRUE(cache_lru.exists(7));
     EXPECT_EQ(777, cache_lru.get(7));
-    EXPECT_EQ(1u, cache_lru.size());
+    EXPECT_EQ(1u, cache_lru.cost());
 }
 
 TEST(CacheTest, OverwritingPut) {
@@ -51,7 +51,7 @@ TEST(CacheTest, OverwritingPut) {
     cache_lru.put(7, 222);
     EXPECT_TRUE(cache_lru.exists(7));
     EXPECT_EQ(222, cache_lru.get(7));
-    EXPECT_EQ(1u, cache_lru.size());
+    EXPECT_EQ(1u, cache_lru.cost());
 }
 
 TEST(CacheTest, MissingValue) {
@@ -66,13 +66,13 @@ TEST(CacheTest, DropValue) {
     cache_lru.put(7, 777);
     cache_lru.put(8, 888);
     cache_lru.put(9, 999);
-    EXPECT_EQ(3u, cache_lru.size());
+    EXPECT_EQ(3u, cache_lru.cost());
     EXPECT_TRUE(cache_lru.exists(7));
     EXPECT_EQ(777, cache_lru.get(7));
 
     EXPECT_TRUE(cache_lru.drop(7));
 
-    EXPECT_EQ(2u, cache_lru.size());
+    EXPECT_EQ(2u, cache_lru.cost());
     EXPECT_FALSE(cache_lru.exists(7));
     EXPECT_THROW(cache_lru.get(7).value(), std::range_error);
 
@@ -91,25 +91,25 @@ TEST(CacheTest, VariableCost) {
     cache_lru.put(1, 11);
     cache_lru.put(2, 22);
     cache_lru.put(3, 33);
-    EXPECT_EQ(66u, cache_lru.size());
+    EXPECT_EQ(66u, cache_lru.cost());
 
     cache_lru.put(4, 44);
-    EXPECT_EQ(99u, cache_lru.size());
+    EXPECT_EQ(99u, cache_lru.cost());
     EXPECT_FALSE(cache_lru.exists(1));
     EXPECT_TRUE(cache_lru.exists(2));
     EXPECT_TRUE(cache_lru.exists(3));
     EXPECT_TRUE(cache_lru.exists(4));
 
     cache_lru.put(5, 55);
-    EXPECT_EQ(99u, cache_lru.size());
+    EXPECT_EQ(99u, cache_lru.cost());
     EXPECT_FALSE(cache_lru.exists(1));
     EXPECT_FALSE(cache_lru.exists(2));
     EXPECT_FALSE(cache_lru.exists(3));
     EXPECT_TRUE(cache_lru.exists(4));
     EXPECT_TRUE(cache_lru.exists(5));
 
     cache_lru.put(1, 11);
-    EXPECT_EQ(66u, cache_lru.size());
+    EXPECT_EQ(66u, cache_lru.cost());
     EXPECT_TRUE(cache_lru.exists(1));
     EXPECT_FALSE(cache_lru.exists(2));
     EXPECT_FALSE(cache_lru.exists(3));
@@ -121,22 +121,22 @@ TEST(CacheTest, TooBigValue) {
     zim::lru_cache<size_t, size_t, IdCost> cache_lru(10);
 
     cache_lru.put(1, 11);
-    EXPECT_EQ(11u, cache_lru.size());
+    EXPECT_EQ(11u, cache_lru.cost());
     EXPECT_TRUE(cache_lru.exists(1));
 
     cache_lru.put(2, 22);
-    EXPECT_EQ(22u, cache_lru.size());
+    EXPECT_EQ(22u, cache_lru.cost());
     EXPECT_FALSE(cache_lru.exists(1));
     EXPECT_TRUE(cache_lru.exists(2));
 
     cache_lru.put(3, 33);
-    EXPECT_EQ(33u, cache_lru.size());
+    EXPECT_EQ(33u, cache_lru.cost());
     EXPECT_FALSE(cache_lru.exists(1));
     EXPECT_FALSE(cache_lru.exists(2));
     EXPECT_TRUE(cache_lru.exists(3));
 
     cache_lru.put(1, 11);
-    EXPECT_EQ(11u, cache_lru.size());
+    EXPECT_EQ(11u, cache_lru.cost());
     EXPECT_TRUE(cache_lru.exists(1));
     EXPECT_FALSE(cache_lru.exists(2));
     EXPECT_FALSE(cache_lru.exists(3));
@@ -166,7 +166,7 @@ TEST(CacheTest1, KeepsAllValuesWithinCapacity) {
 
     EXPECT_RANGE_FULLY_IN_CACHE(cache_lru, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY), NUM_OF_TEST2_RECORDS, 1)
 
-    size_t size = cache_lru.size();
+    size_t size = cache_lru.cost();
     EXPECT_EQ(TEST2_CACHE_CAPACITY, size);
 }
 
@@ -177,20 +177,20 @@ TEST(CacheTest1, ChangeCacheCapacity) {
         cache_lru.put(i, i);
     }
 
-    EXPECT_EQ(TEST2_CACHE_CAPACITY, cache_lru.size());
+    EXPECT_EQ(TEST2_CACHE_CAPACITY, cache_lru.cost());
     EXPECT_RANGE_MISSING_FROM_CACHE(cache_lru, 0, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY))
     EXPECT_RANGE_FULLY_IN_CACHE(cache_lru, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY), NUM_OF_TEST2_RECORDS, 1)
 
-    cache_lru.setMaxSize(TEST2_CACHE_CAPACITY_SMALL);
-    EXPECT_EQ(TEST2_CACHE_CAPACITY_SMALL, cache_lru.size());
+    cache_lru.setMaxCost(TEST2_CACHE_CAPACITY_SMALL);
+    EXPECT_EQ(TEST2_CACHE_CAPACITY_SMALL, cache_lru.cost());
     EXPECT_RANGE_MISSING_FROM_CACHE(cache_lru, 0, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY_SMALL))
     EXPECT_RANGE_FULLY_IN_CACHE(cache_lru, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY_SMALL), NUM_OF_TEST2_RECORDS, 1)
 
-    cache_lru.setMaxSize(TEST2_CACHE_CAPACITY);
+    cache_lru.setMaxCost(TEST2_CACHE_CAPACITY);
     for (int i = 0; i < NUM_OF_TEST2_RECORDS; ++i) {
         cache_lru.put(i, 1000*i);
     }
-    EXPECT_EQ(TEST2_CACHE_CAPACITY, cache_lru.size());
+    EXPECT_EQ(TEST2_CACHE_CAPACITY, cache_lru.cost());
     EXPECT_RANGE_MISSING_FROM_CACHE(cache_lru, 0, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY))
     EXPECT_RANGE_FULLY_IN_CACHE(cache_lru, (NUM_OF_TEST2_RECORDS - TEST2_CACHE_CAPACITY), NUM_OF_TEST2_RECORDS, 1000)
 }