Move mutex acquisition outside write transaction (#4322)

nanocurrency · Nov 5, 2023 · 7bf9790 · 7bf9790
1 parent 6244918
commit 7bf9790
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 16 deletions.
diff --git a/nano/node/voting.cpp b/nano/node/voting.cpp
@@ -184,7 +184,7 @@ nano::vote_generator::~vote_generator ()
 	stop ();
 }
 
-void nano::vote_generator::process (store::write_transaction const & transaction, nano::root const & root_a, nano::block_hash const & hash_a)
+bool nano::vote_generator::should_vote (store::write_transaction const & transaction, nano::root const & root_a, nano::block_hash const & hash_a)
 {
 	bool should_vote = false;
 	if (is_final)
@@ -198,16 +198,7 @@ void nano::vote_generator::process (store::write_transaction const & transaction
 		auto block (ledger.store.block.get (transaction, hash_a));
 		should_vote = block != nullptr && ledger.dependents_confirmed (transaction, *block);
 	}
-	if (should_vote)
-	{
-		nano::unique_lock<nano::mutex> lock{ mutex };
-		candidates.emplace_back (root_a, hash_a);
-		if (candidates.size () >= nano::network::confirm_ack_hashes_max)
-		{
-			lock.unlock ();
-			condition.notify_all ();
-		}
-	}
+	return should_vote;
 }
 
 void nano::vote_generator::start ()
@@ -241,11 +232,28 @@ void nano::vote_generator::add (const root & root, const block_hash & hash)
 
 void nano::vote_generator::process_batch (std::deque<queue_entry_t> & batch)
 {
-	auto transaction = ledger.store.tx_begin_write ({ tables::final_votes });
+	std::deque<candidate_t> candidates_new;
+	{
+		auto transaction = ledger.store.tx_begin_write ({ tables::final_votes });
 
-	for (auto & [root, hash] : batch)
+		for (auto & [root, hash] : batch)
+		{
+			if (should_vote (transaction, root, hash))
+			{
+				candidates_new.emplace_back (root, hash);
+			}
+		}
+		// Commit write transaction
+	}
+	if (!candidates_new.empty ())
 	{
-		process (transaction, root, hash);
+		nano::unique_lock<nano::mutex> lock{ mutex };
+		candidates.insert (candidates.end (), candidates_new.begin (), candidates_new.end ());
+		if (candidates.size () >= nano::network::confirm_ack_hashes_max)
+		{
+			lock.unlock ();
+			condition.notify_all ();
+		}
 	}
 }
 

diff --git a/nano/node/voting.hpp b/nano/node/voting.hpp
@@ -141,10 +141,11 @@ class vote_generator final
 	void broadcast_action (std::shared_ptr<nano::vote> const &) const;
 	void process_batch (std::deque<queue_entry_t> & batch);
 	/**
-	 * Check if block is eligible for vote generation, then generates a vote or broadcasts votes already in cache
+	 * Check if block is eligible for vote generation
 	 * @param transaction : needs `tables::final_votes` lock
+	 * @return: Should vote
 	 */
-	void process (store::write_transaction const &, nano::root const &, nano::block_hash const &);
+	bool should_vote (store::write_transaction const &, nano::root const &, nano::block_hash const &);
 
 private:
 	std::function<void (std::shared_ptr<nano::vote> const &, std::shared_ptr<nano::transport::channel> &)> reply_action; // must be set only during initialization by using set_reply_action