Merge pull request #4636 from pwojcikdev/revert-activate-develop

Optimize election scheduler activate function

nano/lib/numbers.hpp

@@ -62,6 +62,11 @@ class amount : public uint128_union
 {
 public:
 	using uint128_union::uint128_union;
+
+	operator nano::uint128_t () const
+	{
+		return number ();
+	}
 };
 class raw_key;
 

nano/lib/stats_enums.hpp

@@ -363,6 +363,9 @@ enum class detail
 
 	// backlog
 	activated,
+	activate_failed,
+	activate_skip,
+	activate_full,
 
 	// active
 	insert,

nano/node/backlog_population.cpp

@@ -8,8 +8,9 @@
 #include <nano/store/component.hpp>
 #include <nano/store/confirmation_height.hpp>
 
-nano::backlog_population::backlog_population (const config & config_a, nano::ledger & ledger, nano::stats & stats_a) :
+nano::backlog_population::backlog_population (const config & config_a, nano::scheduler::component & schedulers, nano::ledger & ledger, nano::stats & stats_a) :
 	config_m{ config_a },
+	schedulers{ schedulers },
 	ledger{ ledger },
 	stats{ stats_a }
 {
@@ -104,7 +105,9 @@ void nano::backlog_population::populate_backlog (nano::unique_lock<nano::mutex>
 				stats.inc (nano::stat::type::backlog, nano::stat::detail::total);
 
 				auto const & account = i->first;
-				activate (transaction, account);
+				auto const & account_info = i->second;
+				activate (transaction, account, account_info);
+
 				next = account.number () + 1;
 			}
 			done = ledger.store.account.begin (transaction, next) == end;
@@ -117,17 +120,8 @@ void nano::backlog_population::populate_backlog (nano::unique_lock<nano::mutex>
 	}
 }
 
-void nano::backlog_population::activate (secure::transaction const & transaction, nano::account const & account)
+void nano::backlog_population::activate (secure::transaction const & transaction, nano::account const & account, nano::account_info const & account_info)
 {
-	debug_assert (!activate_callback.empty ());
-
-	auto const maybe_account_info = ledger.store.account.get (transaction, account);
-	if (!maybe_account_info)
-	{
-		return;
-	}
-	auto const account_info = *maybe_account_info;
-
 	auto const maybe_conf_info = ledger.store.confirmation_height.get (transaction, account);
 	auto const conf_info = maybe_conf_info.value_or (nano::confirmation_height_info{});
 
@@ -137,5 +131,8 @@ void nano::backlog_population::activate (secure::transaction const & transaction
 		stats.inc (nano::stat::type::backlog, nano::stat::detail::activated);
 
 		activate_callback.notify (transaction, account);
+
+		schedulers.optimistic.activate (account, account_info, conf_info);
+		schedulers.priority.activate (transaction, account, account_info, conf_info);
 	}
 }

nano/node/backlog_population.hpp

@@ -3,6 +3,7 @@
 #include <nano/lib/locks.hpp>
 #include <nano/lib/numbers.hpp>
 #include <nano/lib/observer_set.hpp>
+#include <nano/node/scheduler/component.hpp>
 #include <nano/secure/common.hpp>
 
 #include <condition_variable>
@@ -34,7 +35,7 @@ class backlog_population final
 		unsigned frequency;
 	};
 
-	backlog_population (const config &, ledger &, nano::stats &);
+	backlog_population (const config &, nano::scheduler::component &, nano::ledger &, nano::stats &);
 	~backlog_population ();
 
 	void start ();
@@ -54,6 +55,7 @@ class backlog_population final
 	callback_t activate_callback;
 
 private: // Dependencies
+	nano::scheduler::component & schedulers;
 	nano::ledger & ledger;
 	nano::stats & stats;
 
@@ -64,7 +66,7 @@ class backlog_population final
 	bool predicate () const;
 
 	void populate_backlog (nano::unique_lock<nano::mutex> & lock);
-	void activate (secure::transaction const &, nano::account const &);
+	void activate (secure::transaction const &, nano::account const &, nano::account_info const &);
 
 	/** This is a manual trigger, the ongoing backlog population does not use this.
 	 *  It can be triggered even when backlog population (frontiers confirmation) is disabled. */

nano/node/node.cpp

@@ -209,7 +209,7 @@ nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, std::filesy
 	aggregator_impl{ std::make_unique<nano::request_aggregator> (config.request_aggregator, *this, stats, generator, final_generator, history, ledger, wallets, vote_router) },
 	aggregator{ *aggregator_impl },
 	wallets (wallets_store.init_error (), *this),
-	backlog{ nano::backlog_population_config (config), ledger, stats },
+	backlog{ nano::backlog_population_config (config), scheduler, ledger, stats },
 	ascendboot{ config, block_processor, ledger, network, stats },
 	websocket{ config.websocket_config, observers, wallets, ledger, io_ctx, logger },
 	epoch_upgrader{ *this, ledger, store, network_params, logger },
@@ -232,11 +232,6 @@ nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, std::filesy
 		return ledger.weight (rep);
 	};
 
-	backlog.activate_callback.add ([this] (secure::transaction const & transaction, nano::account const & account) {
-		scheduler.priority.activate (transaction, account);
-		scheduler.optimistic.activate (transaction, account);
-	});
-
 	vote_router.vote_processed.add ([this] (std::shared_ptr<nano::vote> const & vote, nano::vote_source source, std::unordered_map<nano::block_hash, nano::vote_code> const & results) {
 		if (source != nano::vote_source::cache)
 		{

nano/node/scheduler/bucket.cpp

@@ -34,14 +34,18 @@ void nano::scheduler::bucket::pop ()
 	queue.erase (queue.begin ());
 }
 
-void nano::scheduler::bucket::push (uint64_t time, std::shared_ptr<nano::block> block)
+// Returns true if the block was inserted
+bool nano::scheduler::bucket::push (uint64_t time, std::shared_ptr<nano::block> block)
 {
-	queue.insert ({ time, block });
+	auto [it, inserted] = queue.insert ({ time, block });
+	release_assert (!queue.empty ());
+	bool was_last = (it == --queue.end ());
 	if (queue.size () > maximum)
 	{
-		debug_assert (!queue.empty ());
 		queue.erase (--queue.end ());
+		return inserted && !was_last;
 	}
+	return inserted;
 }
 
 size_t nano::scheduler::bucket::size () const

nano/node/scheduler/bucket.hpp

@@ -34,7 +34,7 @@ class bucket final
 
 	std::shared_ptr<nano::block> top () const;
 	void pop ();
-	void push (uint64_t time, std::shared_ptr<nano::block> block);
+	bool push (uint64_t time, std::shared_ptr<nano::block> block);
 	size_t size () const;
 	bool empty () const;
 	void dump () const;

nano/node/scheduler/buckets.cpp

@@ -69,15 +69,16 @@ nano::scheduler::buckets::~buckets ()
  * Push a block and its associated time into the prioritization container.
  * The time is given here because sideband might not exist in the case of state blocks.
  */
-void nano::scheduler::buckets::push (uint64_t time, std::shared_ptr<nano::block> block, nano::amount const & priority)
+bool nano::scheduler::buckets::push (uint64_t time, std::shared_ptr<nano::block> block, nano::amount const & priority)
 {
 	auto was_empty = empty ();
 	auto & bucket = find_bucket (priority.number ());
-	bucket.push (time, block);
+	bool added = bucket.push (time, block);
 	if (was_empty)
 	{
 		seek ();
 	}
+	return added;
 }
 
 /** Return the highest priority block of the current bucket */

nano/node/scheduler/buckets.hpp

@@ -42,7 +42,8 @@ class buckets final
 public:
 	buckets (uint64_t maximum = 250000u);
 	~buckets ();
-	void push (uint64_t time, std::shared_ptr<nano::block> block, nano::amount const & priority);
+	// Returns true if the block was inserted
+	bool push (uint64_t time, std::shared_ptr<nano::block> block, nano::amount const & priority);
 	std::shared_ptr<nano::block> top () const;
 	void pop ();
 	std::size_t size () const;

nano/node/scheduler/optimistic.cpp

@@ -55,31 +55,30 @@ void nano::scheduler::optimistic::notify ()
 	condition.notify_all ();
 }
 
-bool nano::scheduler::optimistic::activate_predicate (nano::secure::transaction const & transaction, nano::account const & account) const
+bool nano::scheduler::optimistic::activate_predicate (const nano::account_info & account_info, const nano::confirmation_height_info & conf_info) const
 {
-	auto unconfirmed_height = ledger.any.account_height (transaction, account);
-	auto confirmed_height = ledger.confirmed.account_height (transaction, account);
-	// Account with nothing confirmed yet
-	if (confirmed_height == 0)
+	// Chain with a big enough gap between account frontier and confirmation frontier
+	if (account_info.block_count - conf_info.height > config.gap_threshold)
 	{
 		return true;
 	}
-	// Chain with a big enough gap between account frontier and confirmation frontier
-	if (unconfirmed_height - confirmed_height > config.gap_threshold)
+	// Account with nothing confirmed yet
+	if (conf_info.height == 0)
 	{
 		return true;
 	}
 	return false;
 }
 
-bool nano::scheduler::optimistic::activate (nano::secure::transaction const & transaction, nano::account const & account)
+bool nano::scheduler::optimistic::activate (const nano::account & account, const nano::account_info & account_info, const nano::confirmation_height_info & conf_info)
 {
 	if (!config.enabled)
 	{
 		return false;
 	}
 
-	if (activate_predicate (transaction, account))
+	debug_assert (account_info.block_count >= conf_info.height);
+	if (activate_predicate (account_info, conf_info))
 	{
 		{
 			nano::lock_guard<nano::mutex> lock{ mutex };

nano/node/scheduler/optimistic.hpp

@@ -60,7 +60,7 @@ class optimistic final
 	/**
 	 * Called from backlog population to process accounts with unconfirmed blocks
 	 */
-	bool activate (nano::secure::transaction const & transaction, nano::account const & account);
+	bool activate (nano::account const &, nano::account_info const &, nano::confirmation_height_info const &);
 
 	/**
 	 * Notify about changes in AEC vacancy
@@ -70,7 +70,7 @@ class optimistic final
 	std::unique_ptr<container_info_component> collect_container_info (std::string const & name) const;
 
 private:
-	bool activate_predicate (nano::secure::transaction const & transaction, nano::account const & account) const;
+	bool activate_predicate (nano::account_info const &, nano::confirmation_height_info const &) const;
 
 	bool predicate () const;
 	void run ();

nano/node/scheduler/priority.cpp

@@ -50,31 +50,60 @@ void nano::scheduler::priority::stop ()
 bool nano::scheduler::priority::activate (secure::transaction const & transaction, nano::account const & account)
 {
 	debug_assert (!account.is_zero ());
-	auto head = node.ledger.confirmed.account_head (transaction, account);
-	if (node.ledger.any.account_head (transaction, account) == head)
+	auto info = node.ledger.any.account_get (transaction, account);
+	if (info)
 	{
-		return false;
+		nano::confirmation_height_info conf_info;
+		node.store.confirmation_height.get (transaction, account, conf_info);
+		if (conf_info.height < info->block_count)
+		{
+			return activate (transaction, account, *info, conf_info);
+		}
 	}
-	auto block = node.ledger.any.block_get (transaction, node.ledger.any.block_successor (transaction, { head.is_zero () ? static_cast<nano::uint256_union> (account) : head, head }).value ());
-	if (!node.ledger.dependents_confirmed (transaction, *block))
+	stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::activate_skip);
+	return false; // Not activated
+}
+
+bool nano::scheduler::priority::activate (secure::transaction const & transaction, nano::account const & account, nano::account_info const & account_info, nano::confirmation_height_info const & conf_info)
+{
+	debug_assert (conf_info.frontier != account_info.head);
+
+	auto hash = conf_info.height == 0 ? account_info.open_block : node.ledger.any.block_successor (transaction, conf_info.frontier).value ();
+	auto block = node.ledger.any.block_get (transaction, hash);
+	release_assert (block != nullptr);
+
+	if (node.ledger.dependents_confirmed (transaction, *block))
 	{
-		return false;
-	}
-	auto const balance_priority = std::max (block->balance ().number (), node.ledger.confirmed.block_balance (transaction, head).value_or (0).number ());
-	auto const time_priority = !head.is_zero () ? node.ledger.confirmed.block_get (transaction, head)->sideband ().timestamp : nano::seconds_since_epoch (); // New accounts get current timestamp i.e. lowest priority
+		auto const balance = block->balance ();
+		auto const previous_balance = node.ledger.any.block_balance (transaction, conf_info.frontier).value_or (0);
+		auto const balance_priority = std::max (balance, previous_balance);
 
-	node.stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::activated);
-	node.logger.trace (nano::log::type::election_scheduler, nano::log::detail::block_activated,
-	nano::log::arg{ "account", account.to_account () }, // TODO: Convert to lazy eval
-	nano::log::arg{ "block", block },
-	nano::log::arg{ "time", time_priority },
-	nano::log::arg{ "priority", balance_priority });
+		bool added = false;
+		{
+			nano::lock_guard<nano::mutex> lock{ mutex };
+			added = buckets->push (account_info.modified, block, balance_priority);
+		}
+		if (added)
+		{
+			node.stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::activated);
+			node.logger.trace (nano::log::type::election_scheduler, nano::log::detail::block_activated,
+			nano::log::arg{ "account", account.to_account () }, // TODO: Convert to lazy eval
+			nano::log::arg{ "block", block },
+			nano::log::arg{ "time", account_info.modified },
+			nano::log::arg{ "priority", balance_priority });
 
-	nano::lock_guard<nano::mutex> lock{ mutex };
-	buckets->push (time_priority, block, balance_priority);
-	notify ();
+			notify ();
+		}
+		else
+		{
+			node.stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::activate_full);
+		}
+
+		return true; // Activated
+	}
 
-	return true; // Activated
+	stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::activate_failed);
+	return false; // Not activated
 }
 
 void nano::scheduler::priority::notify ()

nano/node/scheduler/priority.hpp

@@ -12,6 +12,8 @@
 
 namespace nano
 {
+class account_info;
+class confirmation_height_info;
 class block;
 class container_info_component;
 class node;
@@ -48,6 +50,8 @@ class priority final
 	 * @return true if account was activated
 	 */
 	bool activate (secure::transaction const &, nano::account const &);
+	bool activate (secure::transaction const &, nano::account const &, nano::account_info const &, nano::confirmation_height_info const &);
+
 	void notify ();
 	std::size_t size () const;
 	bool empty () const;