[Feature] - Regular CachedResult memory release over time

README.md

@@ -217,11 +217,11 @@ Input commands accepting previous commands' output are:
 For example:
 
 ```bash
-               1st command                  3rd command
-           /                 \                 /  \
-coincenter buy 1500XLM,binance withdraw kraken sell
-                               \             /
-                                 2nd command
+               1st command                        3rd command
+           /                 \                       /  \
+coincenter buy 1500XLM,binance withdraw-apply kraken sell
+                               \                  /
+                                    2nd command
 ```
 
 The 1500XLM will be considered for withdraw from Binance if the buy is completed, and the XLM arrived on Kraken considered for selling when the withdraw completes.

src/tech/include/cachedresult.hpp

@@ -37,13 +37,22 @@ class CachedResultT : public CachedResultBase<typename ClockT::duration> {
   using ResultType = std::remove_cvref_t<decltype(std::declval<T>()(std::declval<FuncTArgs>()...))>;
   using TimePoint = ClockT::time_point;
   using Duration = ClockT::duration;
-  using ResPtrTimePair = std::pair<const ResultType *, TimePoint>;
   using State = CachedResultBase<Duration>::State;
 
  private:
   using TKey = std::tuple<std::remove_cvref_t<FuncTArgs>...>;
-  using TValue = std::pair<ResultType, TimePoint>;
-  using MapType = std::unordered_map<TKey, TValue, HashTuple>;
+
+  struct Value {
+    template <class R>
+    Value(R &&result, TimePoint lastUpdatedTs) : result(std::forward<R>(result)), lastUpdatedTs(lastUpdatedTs) {}
+
+    template <class F, class K>
+    Value(F &func, K &&key, TimePoint lastUpdatedTs)
+        : result(std::apply(func, std::forward<K>(key))), lastUpdatedTs(lastUpdatedTs) {}
+
+    ResultType result;
+    TimePoint lastUpdatedTs;
+  };
 
  public:
   template <class... TArgs>
@@ -56,51 +65,78 @@ class CachedResultT : public CachedResultBase<typename ClockT::duration> {
 
   /// Sets given value associated to the key built with given parameters,
   /// if given timestamp is more recent than the one associated to the value already present at this key (if any)
+  /// refresh period is not checked, if given timestamp is more recent than the one associated to given value, cache
+  /// will be updated.
   template <class ResultTypeT, class... Args>
   void set(ResultTypeT &&val, TimePoint timePoint, Args &&...funcArgs) {
-    auto [it, inserted] = _cachedResultsMap.try_emplace(TKey(std::forward<Args &&>(funcArgs)...),
-                                                        std::forward<ResultTypeT>(val), timePoint);
-    if (!inserted && it->second.second < timePoint) {
-      it->second = TValue(std::forward<ResultTypeT>(val), timePoint);
+    checkPeriodicRehash();
+
+    auto [it, isInserted] = _cachedResultsMap.try_emplace(TKey(std::forward<Args &&>(funcArgs)...),
+                                                          std::forward<ResultTypeT>(val), timePoint);
+    if (!isInserted && it->second.lastUpdatedTs < timePoint) {
+      it->second = Value(std::forward<ResultTypeT>(val), timePoint);
     }
   }
 
   /// Get the latest value associated to the key built with given parameters.
   /// If the value is too old according to refresh period, it will be recomputed automatically.
   template <class... Args>
   const ResultType &get(Args &&...funcArgs) {
-    TKey key(std::forward<Args &&>(funcArgs)...);
-
-    auto nowTime = ClockT::now();
-
-    auto flattenTuple = [this](auto &&...values) { return _func(std::forward<decltype(values) &&>(values)...); };
+    const auto nowTime = ClockT::now();
 
     if (this->_state == State::kForceUniqueRefresh) {
       _cachedResultsMap.clear();
       this->_state = State::kForceCache;
+    } else {
+      checkPeriodicRehash();
     }
 
-    auto it = _cachedResultsMap.find(key);
-    if (it == _cachedResultsMap.end()) {
-      TValue val(std::apply(flattenTuple, key), nowTime);
-      it = _cachedResultsMap.insert_or_assign(std::move(key), std::move(val)).first;
-    } else if (this->_state != State::kForceCache && this->_refreshPeriod < nowTime - it->second.second) {
-      it->second = TValue(std::apply(flattenTuple, std::move(key)), nowTime);
-    }
+    const auto flattenTuple = [this](auto &&...values) { return _func(std::forward<decltype(values) &&>(values)...); };
 
-    return it->second.first;
+    TKey key(std::forward<Args &&>(funcArgs)...);
+    auto [it, isInserted] = _cachedResultsMap.try_emplace(key, flattenTuple, key, nowTime);
+    if (!isInserted && this->_state != State::kForceCache &&
+        this->_refreshPeriod < nowTime - it->second.lastUpdatedTs) {
+      it->second = Value(flattenTuple, std::move(key), nowTime);
+    }
+    return it->second.result;
   }
 
   /// Retrieve a {pointer, lastUpdateTime} to latest value associated to the key built with given parameters.
   /// If no value has been computed for this key, returns a nullptr.
   template <class... Args>
-  ResPtrTimePair retrieve(Args &&...funcArgs) const {
+  std::pair<const ResultType *, TimePoint> retrieve(Args &&...funcArgs) const {
     auto it = _cachedResultsMap.find(TKey(std::forward<Args &&>(funcArgs)...));
-    return it == _cachedResultsMap.end() ? ResPtrTimePair()
-                                         : ResPtrTimePair(std::addressof(it->second.first), it->second.second);
+    if (it == _cachedResultsMap.end()) {
+      return {};
+    }
+    return {std::addressof(it->second.result), it->second.lastUpdatedTs};
   }
 
  private:
+  void checkPeriodicRehash() {
+    static constexpr decltype(this->_flushCounter) kFlushCheckCounter = 20000;
+    if (++this->_flushCounter < kFlushCheckCounter) {
+      return;
+    }
+    this->_flushCounter = 0;
+
+    const auto nowTime = ClockT::now();
+
+    for (auto it = _cachedResultsMap.begin(); it != _cachedResultsMap.end();) {
+      if (this->_refreshPeriod < nowTime - it->second.lastUpdatedTs) {
+        // Data has expired, remove it
+        it = _cachedResultsMap.erase(it);
+      } else {
+        ++it;
+      }
+    }
+
+    _cachedResultsMap.rehash(_cachedResultsMap.size());
+  }
+
+  using MapType = std::unordered_map<TKey, Value, HashTuple>;
+
   T _func;
   MapType _cachedResultsMap;
 };

src/tech/include/cachedresultvault.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <cstdint>
 #include <memory>
 
 #include "cct_type_traits.hpp"
@@ -13,7 +14,7 @@ class CachedResultBase {
   template <class>
   friend class CachedResultVaultT;
 
-  enum class State { kStandardRefresh, kForceUniqueRefresh, kForceCache };
+  enum class State : int8_t { kStandardRefresh, kForceUniqueRefresh, kForceCache };
 
   explicit CachedResultBase(DurationT refreshPeriod) : _refreshPeriod(refreshPeriod) {}
 
@@ -22,6 +23,7 @@ class CachedResultBase {
   void unfreeze() noexcept { _state = State::kStandardRefresh; }
 
   DurationT _refreshPeriod;
+  uint32_t _flushCounter{};
   State _state = State::kStandardRefresh;
 };
 

src/tech/test/cachedresult_test.cpp

@@ -31,22 +31,25 @@ class Incr {
 
 // We use std::chrono::steady_clock for unit test as it is monotonic (system_clock is not)
 // Picking a number that is not too small to avoid issues with slow systems
-constexpr std::chrono::steady_clock::duration kCacheTime = milliseconds(10);
+using SteadyClock = std::chrono::steady_clock;
+
+constexpr SteadyClock::duration kCacheTime = milliseconds(10);
 constexpr auto kCacheExpireTime = kCacheTime + milliseconds(2);
 
 template <class T, class... FuncTArgs>
-using CachedResultSteadyClock = CachedResultT<std::chrono::steady_clock, T, FuncTArgs...>;
+using CachedResultSteadyClock = CachedResultT<SteadyClock, T, FuncTArgs...>;
 
-using CachedResultOptionsSteadyClock = CachedResultOptionsT<std::chrono::steady_clock::duration>;
+using CachedResultOptionsSteadyClock = CachedResultOptionsT<SteadyClock::duration>;
 
-using CachedResultVaultSteadyClock = CachedResultVaultT<std::chrono::steady_clock::duration>;
+using CachedResultVaultSteadyClock = CachedResultVaultT<SteadyClock::duration>;
 
 }  // namespace
 
 class CachedResultTestBasic : public ::testing::Test {
  protected:
   CachedResultVaultSteadyClock vault;
-  CachedResultSteadyClock<Incr> cachedResult{CachedResultOptionsSteadyClock(kCacheTime, vault)};
+  SteadyClock::duration refreshTime{kCacheTime};
+  CachedResultSteadyClock<Incr> cachedResult{CachedResultOptionsSteadyClock(refreshTime, vault)};
 };
 
 TEST_F(CachedResultTestBasic, GetCache) {
@@ -56,6 +59,8 @@ TEST_F(CachedResultTestBasic, GetCache) {
   std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(), 2);
   EXPECT_EQ(cachedResult.get(), 2);
+  std::this_thread::sleep_for(kCacheExpireTime);
+  EXPECT_EQ(cachedResult.get(), 3);
 }
 
 TEST_F(CachedResultTestBasic, Freeze) {
@@ -74,7 +79,8 @@ class CachedResultTest : public ::testing::Test {
  protected:
   using CachedResType = CachedResultSteadyClock<Incr, int, int>;
 
-  CachedResType cachedResult{CachedResultOptionsSteadyClock(kCacheTime)};
+  SteadyClock::duration refreshTime{kCacheTime};
+  CachedResType cachedResult{CachedResultOptionsSteadyClock(refreshTime)};
 };
 
 TEST_F(CachedResultTest, GetCache) {
@@ -86,25 +92,67 @@ TEST_F(CachedResultTest, GetCache) {
 }
 
 TEST_F(CachedResultTest, SetInCache) {
-  auto nowTime = std::chrono::steady_clock::now();
+  auto nowTime = SteadyClock::now();
   cachedResult.set(42, nowTime, 3, 4);
+
   EXPECT_EQ(cachedResult.get(3, 4), 42);
   EXPECT_EQ(cachedResult.get(3, 4), 42);
   std::this_thread::sleep_for(kCacheExpireTime);
   EXPECT_EQ(cachedResult.get(3, 4), 7);
   cachedResult.set(42, nowTime, 3, 4);  // timestamp too old, should not be set
   EXPECT_EQ(cachedResult.get(3, 4), 7);
+
+  cachedResult.set(42, nowTime + 2 * kCacheExpireTime, 3, 4);  // should be set
+  EXPECT_EQ(cachedResult.get(3, 4), 42);
 }
 
 TEST_F(CachedResultTest, RetrieveFromCache) {
-  using RetrieveRetType = CachedResType::ResPtrTimePair;
+  auto [ptr, ts] = cachedResult.retrieve(-5, 3);
+
+  EXPECT_EQ(ptr, nullptr);
+  EXPECT_EQ(ts, SteadyClock::time_point{});
 
-  EXPECT_EQ(cachedResult.retrieve(-5, 3), RetrieveRetType());
   EXPECT_EQ(cachedResult.get(-5, 3), -2);
-  RetrieveRetType ret = cachedResult.retrieve(-5, 3);
-  ASSERT_NE(ret.first, nullptr);
-  EXPECT_EQ(*ret.first, -2);
-  EXPECT_GT(ret.second, std::chrono::steady_clock::time_point());
-  EXPECT_EQ(cachedResult.retrieve(-4, 3), RetrieveRetType());
+  std::tie(ptr, ts) = cachedResult.retrieve(-5, 3);
+  ASSERT_NE(ptr, nullptr);
+  EXPECT_EQ(*ptr, -2);
+  EXPECT_GT(ts, SteadyClock::time_point());
+
+  std::tie(ptr, ts) = cachedResult.retrieve(-4, 3);
+  EXPECT_EQ(ptr, nullptr);
+  EXPECT_EQ(ts, SteadyClock::time_point{});
+}
+
+class CachedResultTestZeroRefreshTime : public ::testing::Test {
+ protected:
+  using CachedResType = CachedResultSteadyClock<Incr, int, int>;
+
+  SteadyClock::duration refreshTime{};
+  CachedResType cachedResult{CachedResultOptionsSteadyClock(refreshTime)};
+};
+
+TEST_F(CachedResultTestZeroRefreshTime, GetNoCache) {
+  EXPECT_EQ(cachedResult.get(3, 4), 7);
+  EXPECT_EQ(cachedResult.get(3, 4), 14);
+  EXPECT_EQ(cachedResult.get(3, 4), 21);
+  std::this_thread::sleep_for(kCacheExpireTime);
+  EXPECT_EQ(cachedResult.get(3, 2), 26);
 }
+
+class CachedResultTestMaxRefreshTime : public ::testing::Test {
+ protected:
+  using CachedResType = CachedResultSteadyClock<Incr, int, int>;
+
+  SteadyClock::duration refreshTime{SteadyClock::duration::max()};
+  CachedResType cachedResult{CachedResultOptionsSteadyClock(refreshTime)};
+};
+
+TEST_F(CachedResultTestMaxRefreshTime, GetCache) {
+  EXPECT_EQ(cachedResult.get(3, 4), 7);
+  EXPECT_EQ(cachedResult.get(3, 4), 7);
+  EXPECT_EQ(cachedResult.get(3, 4), 7);
+  std::this_thread::sleep_for(kCacheExpireTime);
+  EXPECT_EQ(cachedResult.get(3, 4), 7);
+}
+
 }  // namespace cct