Merge branch 'get_coinbase_hash_cached_refactoring' into develop

contrib/epee/include/cache_helper.h

@@ -61,7 +61,7 @@ namespace epee
     class cache_base
     {
       uint64_t max_allowed_elements;
-      std::list<t_key> most_recet_acessed;
+      std::list<t_key> most_recent_accessed;
       typename container_selector<is_ordered_container, t_key, std::pair<t_value, typename std::list<t_key>::iterator> >::container data;
     protected:
       critical_section m_lock;
@@ -78,7 +78,7 @@ namespace epee
 
       size_t most_recent_accessed_container_size() const
       {
-        return most_recet_acessed.size();
+        return most_recent_accessed.size();
       }
 
       uint64_t get_max_elements() const
@@ -98,7 +98,11 @@ namespace epee
         if (it == data.end())
           return false;
 
-        most_recet_acessed.splice(most_recet_acessed.begin(), most_recet_acessed, it->second.second);
+        //most_recent_accessed.splice(most_recent_accessed.begin(), most_recent_accessed, it->second.second);
+        most_recent_accessed.erase(it->second.second);
+        most_recent_accessed.push_front(k);
+        it->second.second = most_recent_accessed.begin();
+
         v = it->second.first;
         return true;
       }
@@ -106,8 +110,18 @@ namespace epee
       bool set(const t_key& k, const t_value& v)
       {
         CRITICAL_REGION_LOCAL(m_lock);
-        most_recet_acessed.push_front(k);
-        data[k] = std::pair<t_value, typename std::list<t_key>::iterator>(v, most_recet_acessed.begin());
+        auto it = data.find(k);
+        if (it == data.end())
+        {
+          most_recent_accessed.push_front(k);
+          data.insert(std::make_pair(k, std::make_pair(v, most_recent_accessed.begin())));
+        }
+        else
+        {
+          most_recent_accessed.erase(it->second.second);
+          most_recent_accessed.push_front(k);
+          it->second.second = most_recent_accessed.begin();
+        }
 
         trim();
         return true;
@@ -117,7 +131,7 @@ namespace epee
       {
         CRITICAL_REGION_LOCAL(m_lock);
         data.clear();
-        most_recet_acessed.clear();
+        most_recent_accessed.clear();
       }
 
       bool erase(const t_key& k)
@@ -127,20 +141,21 @@ namespace epee
         if (data_it == data.end())
           return false;
 
-        most_recet_acessed.erase(data_it->second.second);
+        most_recent_accessed.erase(data_it->second.second);
         data.erase(data_it);
         return true;
       }
+
     protected:
       void trim()
       {
         CRITICAL_REGION_LOCAL(m_lock);
-        while (most_recet_acessed.size() > max_allowed_elements)
+        while (most_recent_accessed.size() > max_allowed_elements)
         {
-          auto data_it = data.find(most_recet_acessed.back());
+          auto data_it = data.find(most_recent_accessed.back());
           if (data_it != data.end())
             data.erase(data_it);
-          most_recet_acessed.erase(--most_recet_acessed.end());  
+          most_recent_accessed.erase(--most_recent_accessed.end());  
         }
       }
     };
@@ -248,7 +263,10 @@ namespace epee
 
       bool erase(const t_key& k)
       {
-        return m_isolation.isolated_write_access<bool>([&](){return base_class::erase(k); });
+        return m_isolation.isolated_write_access<bool>([&]()
+        {
+          return base_class::erase(k);
+        });
       }
     };
 

contrib/epee/include/misc_language.h

@@ -627,6 +627,18 @@ namespace misc_utils
   };
 
 
+  // helper class mainly intended for using with std::atomic to repair copy-construction in classes where std::atomic is aggregated
+  template<typename parent_t>
+  struct void_copy : public parent_t
+  {
+    void_copy()                                 = default;
+    void_copy(void_copy&&) noexcept             = default;
+    void_copy& operator=(void_copy&&) noexcept  = default;
+
+    void_copy(const void_copy&) : parent_t{}    {}
+    void_copy& operator=(const void_copy&)      { return *this; }
+  };
+
 } // namespace misc_utils
 } // namespace epee
 

src/currency_core/blockchain_storage.cpp

@@ -3838,7 +3838,14 @@ bool blockchain_storage::find_blockchain_supplement(const std::list<crypto::hash
     std::list<crypto::hash> mis;
     get_transactions_direct(m_db_blocks[i]->bl.tx_hashes, blocks.back().second, mis);
     CHECK_AND_ASSERT_MES(!mis.size(), false, "internal error, block " << get_block_hash(m_db_blocks[i]->bl) << " [" << i << "] contains missing transactions: " << mis);
-    blocks.back().third = m_db_transactions.find(get_coinbase_hash_cached(*m_db_blocks[i]));
+    //blocks.back().third = m_db_transactions.find(get_coinbase_hash_cached(*m_db_blocks[i]));
+
+    crypto::hash coinbase_hash = get_coinbase_hash_cached(*m_db_blocks[i]);
+    blocks.back().third = m_db_transactions.find(coinbase_hash);
+    if (!blocks.back().third)
+    {
+      LOG_PRINT_YELLOW("m_db_transactions.find failed for coinbase hash: " << coinbase_hash << ", height: " << i, LOG_LEVEL_0);
+    }
   }
   return true;
 }

src/currency_core/blockchain_storage_basic.h

@@ -77,6 +77,7 @@ namespace currency
     // This is an optional data fields, It is not included in serialization and therefore is never stored in the database.
     // It might be calculated "on the fly" to speed up access operations.
     mutable std::shared_ptr<crypto::hash> m_cache_coinbase_id;
+    mutable epee::misc_utils::void_copy<std::atomic<uint8_t>> m_cache_coinbase_state;
   };
 
   struct gindex_increment
@@ -194,14 +195,31 @@ namespace currency
 
 
   inline crypto::hash get_coinbase_hash_cached(const block_extended_info& bei)
-  {    
-    std::shared_ptr<crypto::hash> local_coinbase_id = bei.m_cache_coinbase_id;
-    if (!local_coinbase_id)
-    {
-      bei.m_cache_coinbase_id = std::make_shared<crypto::hash>(get_transaction_hash(bei.bl.miner_tx));
-      local_coinbase_id = bei.m_cache_coinbase_id;
-    }
-    return *local_coinbase_id;
+  {
+    // bei.m_cache_coinbase_state : 0 -- m_cache_coinbase_id is empty
+    //                              1 -- m_cache_coinbase_id is calculating and writing
+    //                              2 -- m_cache_coinbase_id is ready to read
 
+    if (bei.m_cache_coinbase_state == 2)
+    {
+      // state is 2, cache must be ready, access the cache
+      std::shared_ptr<crypto::hash> local_coinbase_id = bei.m_cache_coinbase_id;
+      CHECK_AND_ASSERT_THROW_MES(local_coinbase_id, "internal error: m_cache_coinbase_id is empty");
+      return *local_coinbase_id;
+    }
+
+    uint8_t state = 0;
+    if (bei.m_cache_coinbase_state.compare_exchange_strong(state, 1))
+    {
+      // state has just been 0, now 1, we're calculating
+      std::shared_ptr<crypto::hash> ptr_h = std::make_shared<crypto::hash>(get_transaction_hash(bei.bl.miner_tx));
+      bei.m_cache_coinbase_id = ptr_h;
+      bei.m_cache_coinbase_state.store(2);
+      return *ptr_h;
+    }
+
+    // state is 1, another thread is calculating, so we calculate locally and don't touch the cache
+    crypto::hash h = get_transaction_hash(bei.bl.miner_tx);
+    return h;
   }
 } // namespace currency