Merge pull request monero-project#6 from 0xFFFC0000/dev/0xfffc/stress…

…_master_startup_speedup src: update internal data structure to boost::bimap.
rbrunner7 · Jun 20, 2024 · 67f4077 · 67f4077
2 parents bd4e603 + dc6d2e7
commit 67f4077
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 21 deletions.
diff --git a/external/CMakeLists.txt b/external/CMakeLists.txt
@@ -35,6 +35,10 @@
 # ...except for FreeBSD, because FreeBSD is a special case that doesn't play well with
 # others.
 
+if(NOT MSVC)              
+  add_compile_options(-D_GNU_SOURCE)
+endif()
+
 find_package(Miniupnpc REQUIRED)
 
 message(STATUS "Using in-tree miniupnpc")

diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
@@ -29,10 +29,13 @@
 // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers
 
 #include <algorithm>
+#include <boost/bimap/support/iterator_type_by.hpp>
 #include <boost/filesystem.hpp>
+#include <cstdlib>
 #include <unordered_set>
 #include <vector>
 
+
 #include "tx_pool.h"
 #include "cryptonote_tx_utils.h"
 #include "cryptonote_basic/cryptonote_boost_serialization.h"
@@ -42,6 +45,7 @@
 #include "blockchain_db/blockchain_db.h"
 #include "int-util.h"
 #include "misc_language.h"
+#include "misc_log_ex.h"
 #include "warnings.h"
 #include "common/perf_timer.h"
 #include "crypto/hash.h"
@@ -457,14 +461,15 @@ namespace cryptonote
     bool changed = false;
 
     // this will never remove the first one, but we don't care
+    //    m_txs_by_fee_and_receive_time.left.end();
     auto it = --m_txs_by_fee_and_receive_time.end();
     while (it != m_txs_by_fee_and_receive_time.begin())
     {
       if (m_txpool_weight <= bytes)
         break;
       try
       {
-        const crypto::hash &txid = it->second;
+        const crypto::hash &txid = it->get_right();
         txpool_tx_meta_t meta;
         if (!m_blockchain.get_txpool_tx_meta(txid, meta))
         {
@@ -491,11 +496,11 @@ namespace cryptonote
           return;
         }
         // remove first, in case this throws, so key images aren't removed
-        MINFO("Pruning tx " << txid << " from txpool: weight: " << meta.weight << ", fee/byte: " << it->first.first);
+        MINFO("Pruning tx " << txid << " from txpool: weight: " << meta.weight << ", fee/byte: " << it->get_left().first);
         m_blockchain.remove_txpool_tx(txid);
         reduce_txpool_weight(meta.weight);
         remove_transaction_keyimages(tx, txid);
-        MINFO("Pruned tx " << txid << " from txpool: weight: " << meta.weight << ", fee/byte: " << it->first.first);
+        MINFO("Pruned tx " << txid << " from txpool: weight: " << meta.weight << ", fee/byte: " << it->get_left().first);
 
         auto it_prev = it;
         --it_prev;
@@ -629,7 +634,8 @@ namespace cryptonote
       return false;
     }
 
-    remove_tx_from_transient_lists(find_tx_in_sorted_container(id), id, sensitive);
+    auto return_iterator = find_tx_in_sorted_container(id);
+    remove_tx_from_transient_lists(return_iterator, id, sensitive);
     ++m_cookie;
     return true;
   }
@@ -751,13 +757,22 @@ namespace cryptonote
     m_remove_stuck_tx_interval.do_call([this](){return remove_stuck_transactions();});
   }
   //---------------------------------------------------------------------------------
-  sorted_tx_container::iterator tx_memory_pool::find_tx_in_sorted_container(const crypto::hash& id) const
+  cryptonote::sorted_tx_container::iterator tx_memory_pool::find_tx_in_sorted_container(const crypto::hash& id)
   {
-    return std::find_if( m_txs_by_fee_and_receive_time.begin(), m_txs_by_fee_and_receive_time.end()
-                       , [&](const sorted_tx_container::value_type& a){
-                         return a.second == id;
-                       }
-    );
+    // sorted_tx_container::
+      //    sorted_tx_container:
+    //    crypto::hash* non_const_id = const_cast<crypto::hash*>(&id);
+    auto right_iterator = m_txs_by_fee_and_receive_time.right.find(id);
+    //        cryptonote::sorted_tx_container::right_iterator right_iterator = m_txs_by_fee_and_receive_time.right.find<crypto::hash>(*non_const_id);
+    //    sorted_tx_container::right_iterator* new_it = const_cast<sorted_tx_container::right_iterator*>(&right_iterator);
+    //    return m_txs_by_fee_and_receive_time.project_up(*new_it);
+    auto return_iterator = m_txs_by_fee_and_receive_time.project_up(right_iterator);
+    return return_iterator;
+    // return std::find_if( m_txs_by_fee_and_receive_time.begin(), m_txs_by_fee_and_receive_time.end()
+    //                    , [&](const sorted_tx_container::value_type& a){
+    //                      return a.second == id;
+    //                    }
+    // );
   }
   //---------------------------------------------------------------------------------
   //TODO: investigate whether boolean return is appropriate
@@ -1644,15 +1659,15 @@ namespace cryptonote
     for (; sorted_it != m_txs_by_fee_and_receive_time.end(); ++sorted_it)
     {
       txpool_tx_meta_t meta;
-      if (!m_blockchain.get_txpool_tx_meta(sorted_it->second, meta))
+      if (!m_blockchain.get_txpool_tx_meta(sorted_it->get_right(), meta))
       {
         static bool warned = false;
         if (!warned)
-          MERROR("  failed to find tx meta: " << sorted_it->second << " (will only print once)");
+          MERROR("  failed to find tx meta: " << sorted_it->get_right() << " (will only print once)");
         warned = true;
         continue;
       }
-      LOG_PRINT_L2("Considering " << sorted_it->second << ", weight " << meta.weight << ", current block weight " << total_weight << "/" << max_total_weight << ", current coinbase " << print_money(best_coinbase) << ", relay method " << (unsigned)meta.get_relay_method());
+      LOG_PRINT_L2("Considering " << sorted_it->get_right() << ", weight " << meta.weight << ", current block weight " << total_weight << "/" << max_total_weight << ", current coinbase " << print_money(best_coinbase) << ", relay method " << (unsigned)meta.get_relay_method());
 
       if (!meta.matches(relay_category::legacy) && !(m_mine_stem_txes && meta.get_relay_method() == relay_method::stem))
       {
@@ -1702,7 +1717,7 @@ namespace cryptonote
       }
 
       // "local" and "stem" txes are filtered above
-      cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(sorted_it->second, relay_category::all);
+      cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(sorted_it->get_right(), relay_category::all);
 
       cryptonote::transaction tx;
 
@@ -1713,7 +1728,7 @@ namespace cryptonote
       bool ready = false;
       try
       {
-        ready = is_transaction_ready_to_go(meta, sorted_it->second, txblob, tx);
+        ready = is_transaction_ready_to_go(meta, sorted_it->get_right(), txblob, tx);
       }
       catch (const std::exception &e)
       {
@@ -1724,7 +1739,7 @@ namespace cryptonote
       {
         try
 	{
-	  m_blockchain.update_txpool_tx(sorted_it->second, meta);
+	  m_blockchain.update_txpool_tx(sorted_it->get_right(), meta);
 	}
         catch (const std::exception &e)
 	{
@@ -1743,7 +1758,7 @@ namespace cryptonote
         continue;
       }
 
-      bl.tx_hashes.push_back(sorted_it->second);
+      bl.tx_hashes.push_back(sorted_it->get_right());
       total_weight += meta.weight;
       fee += meta.fee;
       best_coinbase = coinbase;
@@ -1845,16 +1860,23 @@ namespace cryptonote
       it->second = now;
 
       auto sorted_it = find_tx_in_sorted_container(txid);
+
       if (sorted_it == m_txs_by_fee_and_receive_time.end())
       {
         MDEBUG("Re-adding tx " << txid << " to tx pool, but it was not found in the sorted txs container");
       }
       else
       {
         m_txs_by_fee_and_receive_time.erase(sorted_it);
+        //        m_txs_by_fee_and_receive_time.erase(sorted_it);
       }
     }
-    m_txs_by_fee_and_receive_time.emplace(std::pair<double, time_t>(fee, receive_time), txid);
+    auto return_value = m_txs_by_fee_and_receive_time.insert(sorted_tx_container::value_type(std::pair<double, time_t>(fee, receive_time), txid));
+
+    if (return_value.second == false) {
+        MERROR("Failed to add txid " << txid << " to the m_txs_by_fee_and_receive_time");
+    }
+    //m_txs_by_fee_and_receive_time.emplace(std::pair<double, time_t>(fee, receive_time), txid);
 
     // Don't check for "resurrected" txs in case of reorgs i.e. don't check in 'm_removed_txs_by_time'
     // whether we have that txid there and if yes remove it; this results in possible duplicates
@@ -1868,13 +1890,15 @@ namespace cryptonote
   //---------------------------------------------------------------------------------
   void tx_memory_pool::remove_tx_from_transient_lists(const cryptonote::sorted_tx_container::iterator& sorted_it, const crypto::hash& txid, bool sensitive)
   {
+    //    cryptonote::sorted_tx_container::left_const_iterator
+    //     cryptonote::sorted_tx_container::iterator
     if (sorted_it == m_txs_by_fee_and_receive_time.end())
     {
       LOG_PRINT_L1("Removing tx " << txid << " from tx pool, but it was not found in the sorted txs container!");
     }
     else
     {
-      m_txs_by_fee_and_receive_time.erase(sorted_it);
+           m_txs_by_fee_and_receive_time.erase(sorted_it);
     }
 
     const std::unordered_map<crypto::hash, time_t>::iterator it = m_added_txs_by_id.find(txid);

diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h
@@ -39,6 +39,9 @@
 #include <queue>
 #include <boost/serialization/version.hpp>
 #include <boost/utility.hpp>
+#include <boost/bimap.hpp>
+#include <boost/bimap/set_of.hpp>
+#include <boost/bimap/multiset_of.hpp>
 
 #include "span.h"
 #include "string_tools.h"
@@ -78,8 +81,35 @@ namespace cryptonote
     }
   };
 
+  class firstCompare
+  {
+  public:
+    bool operator()(const std::pair<double, std::time_t>& a, const std::pair<double, std::time_t>& b) const
+    {
+      // sort by greatest first, not least
+      if (a.first > b.first) return true;
+      if (a.first < b.first) return false;
+
+      if (a.second < b.second) return true;
+      return false;
+    }
+  }; 
+
+  class secondCompare
+  {
+  public:
+    bool operator()(const crypto::hash& a, const crypto::hash& b) const
+    {
+      return memcmp(a.data, b.data, sizeof(crypto::hash)) < 0;
+    }
+  };
+
   //! container for sorting transactions by fee per unit size
-  typedef std::set<tx_by_fee_and_receive_time_entry, txCompare> sorted_tx_container;
+  // typedef std::set<tx_by_fee_and_receive_time_entry, txCompare> sorted_tx_container;
+//  typedef boost::bimap<tx_by_fee_and_receive_time_entry, txCompare> sorted_tx_container;
+  typedef boost::bimap<boost::bimaps::multiset_of<std::pair<double, std::time_t>, firstCompare>,
+                       boost::bimaps::set_of<crypto::hash, secondCompare>> sorted_tx_container;
+
 
   /**
    * @brief Transaction pool, handles transactions which are not part of a block
@@ -653,7 +683,7 @@ namespace cryptonote
      *
      * @return an iterator, possibly to the end of the container if not found
      */
-    sorted_tx_container::iterator find_tx_in_sorted_container(const crypto::hash& id) const;
+    cryptonote::sorted_tx_container::iterator find_tx_in_sorted_container(const crypto::hash& id);
 
     //! cache/call Blockchain::check_tx_inputs results
     bool check_tx_inputs(const std::function<cryptonote::transaction&(void)> &get_tx, const crypto::hash &txid, uint64_t &max_used_block_height, crypto::hash &max_used_block_id, tx_verification_context &tvc, bool kept_by_block = false) const;