From d5ab41e3978f18b1db15e47f8786349cd9be6c4c Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Mon, 20 Jan 2020 13:16:39 +0000 Subject: [PATCH] Emplacing wherever possible and other misc enhancements (#2498) * Emplacing wherever possible and other misc enhancements - Using emplace/emplace_back where possible. Had to make explicit constructors for [tcp_]endpoint_attempt and channel wrappers - Looping with range for-loops and using const refs - Avoiding one hash and lookup in ::reachout - Using insert/emplace to insert multiple elements in some places, which is known to be faster for many elements * One case where emplace is incorrect * Keep using push/insert when the element is a smart pointer, as there is no difference * Use const ref in constructors --- nano/lib/stats.cpp | 2 +- nano/lib/threading.cpp | 4 ++-- nano/lib/work.cpp | 7 +++---- nano/node/active_transactions.cpp | 4 ++-- nano/node/election.cpp | 8 ++++---- nano/node/lmdb/lmdb.cpp | 2 +- nano/node/network.cpp | 9 +++------ nano/node/nodeconfig.cpp | 2 +- nano/node/online_reps.cpp | 5 +---- nano/node/repcrawler.cpp | 10 +++++----- nano/node/transport/tcp.cpp | 11 +++++------ nano/node/transport/tcp.hpp | 11 ++++++++++- nano/node/transport/udp.cpp | 11 +++++------ nano/node/transport/udp.hpp | 11 ++++++++++- nano/node/vote_processor.cpp | 6 +++--- nano/node/wallet.cpp | 2 +- nano/node/websocket.cpp | 2 +- nano/qt_system/entry.cpp | 2 +- 18 files changed, 59 insertions(+), 50 deletions(-) diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 32ec1ade..64fe6d77 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -213,7 +213,7 @@ std::shared_ptr nano::stat::get_entry_impl (uint32_t key, size auto entry = entries.find (key); if (entry == entries.end ()) { - res = entries.insert (std::make_pair (key, std::make_shared (capacity, interval))).first->second; + res = entries.emplace (key, std::make_shared (capacity, interval)).first->second; } else { diff --git a/nano/lib/threading.cpp b/nano/lib/threading.cpp index ea04f40a..a996d35a 100644 --- a/nano/lib/threading.cpp +++ b/nano/lib/threading.cpp @@ -111,7 +111,7 @@ io_guard (boost::asio::make_work_guard (io_ctx_a)) nano::thread_attributes::set (attrs); for (auto i (0u); i < service_threads_a; ++i) { - threads.push_back (boost::thread (attrs, [&io_ctx_a]() { + threads.emplace_back (attrs, [&io_ctx_a]() { nano::thread_role::set (nano::thread_role::name::io); try { @@ -135,7 +135,7 @@ io_guard (boost::asio::make_work_guard (io_ctx_a)) throw; #endif } - })); + }); } } diff --git a/nano/lib/work.cpp b/nano/lib/work.cpp index 5cf7b242..b4d7b7df 100644 --- a/nano/lib/work.cpp +++ b/nano/lib/work.cpp @@ -50,12 +50,11 @@ opencl (opencl_a) } for (auto i (0u); i < count; ++i) { - auto thread (boost::thread (attrs, [this, i]() { + threads.emplace_back (attrs, [this, i]() { nano::thread_role::set (nano::thread_role::name::work); nano::work_thread_reprioritize (); loop (i); - })); - threads.push_back (std::move (thread)); + }); } } @@ -204,7 +203,7 @@ void nano::work_pool::generate (nano::root const & root_a, std::function lock (mutex); - pending.push_back ({ root_a, callback_a, difficulty_a }); + pending.emplace_back (root_a, callback_a, difficulty_a); } producer_condition.notify_all (); } diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 5a0a802f..2bac7936 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -542,7 +542,7 @@ bool nano::active_transactions::add (std::shared_ptr block_a, bool error = nano::work_validate (*block_a, &difficulty); release_assert (!error); roots.get ().emplace (nano::conflict_info{ root, difficulty, difficulty, election }); - blocks.insert (std::make_pair (hash, election)); + blocks.emplace (hash, election); adjust_difficulty (hash); election->insert_inactive_votes_cache (hash); } @@ -882,7 +882,7 @@ bool nano::active_transactions::publish (std::shared_ptr block_a) result = election->publish (block_a); if (!result && !election->confirmed) { - blocks.insert (std::make_pair (block_a->hash (), election)); + blocks.emplace (block_a->hash (), election); } } return result; diff --git a/nano/node/election.cpp b/nano/node/election.cpp index a1cc968d..4a36c4e8 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -18,8 +18,8 @@ skip_delay (skip_delay_a), confirmed (false), stopped (false) { - last_votes.insert (std::make_pair (node.network_params.random.not_an_account, nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () })); - blocks.insert (std::make_pair (block_a->hash (), block_a)); + last_votes.emplace (node.network_params.random.not_an_account, nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () }); + blocks.emplace (block_a->hash (), block_a); update_dependent (); } @@ -92,7 +92,7 @@ nano::tally_t nano::election::tally () auto block (blocks.find (item.first)); if (block != blocks.end ()) { - result.insert (std::make_pair (item.second, block->second)); + result.emplace (item.second, block->second); } } return result; @@ -217,7 +217,7 @@ bool nano::election::publish (std::shared_ptr block_a) { if (blocks.find (block_a->hash ()) == blocks.end ()) { - blocks.insert (std::make_pair (block_a->hash (), block_a)); + blocks.emplace (block_a->hash (), block_a); insert_inactive_votes_cache (block_a->hash ()); confirm_if_quorum (); node.network.flood_block (block_a, false); diff --git a/nano/node/lmdb/lmdb.cpp b/nano/node/lmdb/lmdb.cpp index 4fef14f3..a518ccc4 100644 --- a/nano/node/lmdb/lmdb.cpp +++ b/nano/node/lmdb/lmdb.cpp @@ -324,7 +324,7 @@ void nano::mdb_store::upgrade_v3_to_v4 (nano::write_transaction const & transact { nano::block_hash const & hash (i->first); nano::pending_info_v3 const & info (i->second); - items.push (std::make_pair (nano::pending_key (info.destination, hash), nano::pending_info_v14 (info.source, info.amount, nano::epoch::epoch_0))); + items.emplace (nano::pending_key (info.destination, hash), nano::pending_info_v14 (info.source, info.amount, nano::epoch::epoch_0)); } mdb_drop (env.tx (transaction_a), pending_v0, 0); while (!items.empty ()) diff --git a/nano/node/network.cpp b/nano/node/network.cpp index b9ee24b7..cbceae62 100644 --- a/nano/node/network.cpp +++ b/nano/node/network.cpp @@ -22,7 +22,7 @@ disconnect_observer ([]() {}) nano::thread_attributes::set (attrs); for (size_t i = 0; i < node.config.network_threads; ++i) { - packet_processing_threads.push_back (boost::thread (attrs, [this]() { + packet_processing_threads.emplace_back (attrs, [this]() { nano::thread_role::set (nano::thread_role::name::packet_processing); try { @@ -52,7 +52,7 @@ disconnect_observer ([]() {}) { this->node.logger.try_log ("Exiting packet processing thread"); } - })); + }); } } @@ -265,10 +265,7 @@ void nano::network::broadcast_confirm_req (std::shared_ptr block_a) // broadcast request to all peers (with max limit 2 * sqrt (peers count)) auto peers (node.network.list (std::min (static_cast (100), 2 * node.network.size_sqrt ()))); list->clear (); - for (auto & peer : peers) - { - list->push_back (peer); - } + list->insert (list->end (), peers.begin (), peers.end ()); } /* diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 43fd4bc8..175e9303 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -642,7 +642,7 @@ nano::error nano::node_config::deserialize_json (bool & upgraded_a, nano::jsonco if (!result) { auto address (entry.substr (0, port_position)); - this->work_peers.push_back (std::make_pair (address, port)); + this->work_peers.emplace_back (address, port); } } }); diff --git a/nano/node/online_reps.cpp b/nano/node/online_reps.cpp index 8f0a2801..7dc86f09 100644 --- a/nano/node/online_reps.cpp +++ b/nano/node/online_reps.cpp @@ -79,10 +79,7 @@ std::vector nano::online_reps::list () { std::vector result; nano::lock_guard lock (mutex); - for (auto & i : reps) - { - result.push_back (i); - } + result.insert (result.end (), reps.begin (), reps.end ()); return result; } diff --git a/nano/node/repcrawler.cpp b/nano/node/repcrawler.cpp index 076dc815..52bc519b 100644 --- a/nano/node/repcrawler.cpp +++ b/nano/node/repcrawler.cpp @@ -183,7 +183,7 @@ void nano::rep_crawler::response (std::shared_ptr & ch { if (active.count (*i) != 0) { - responses.push_back (std::make_pair (channel_a, vote_a)); + responses.emplace_back (channel_a, vote_a); break; } } @@ -230,13 +230,13 @@ void nano::rep_crawler::cleanup_reps () { // Check known rep channels nano::lock_guard lock (probable_reps_mutex); - for (auto i (probable_reps.get ().begin ()), n (probable_reps.get ().end ()); i != n; ++i) + for (auto const & rep : probable_reps.get ()) { - channels.push_back (i->channel); + channels.push_back (rep.channel); } } // Remove reps with inactive channels - for (auto i : channels) + for (auto const & i : channels) { bool equal (false); if (i->get_type () == nano::transport::transport_type::tcp) @@ -306,7 +306,7 @@ std::vector> nano::rep_crawler::repres { std::vector> result; auto reps (representatives (count_a)); - for (auto rep : reps) + for (auto const & rep : reps) { result.push_back (rep.channel); } diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index a2051ff5..d66ce9c1 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -118,7 +118,7 @@ bool nano::transport::tcp_channels::insert (std::shared_ptr ().erase (node_id); } - channels.get ().insert ({ channel_a, socket_a, bootstrap_server_a }); + channels.get ().emplace (channel_a, socket_a, bootstrap_server_a); error = false; lock.unlock (); node.network.channel_observer (channel_a); @@ -361,9 +361,8 @@ bool nano::transport::tcp_channels::reachout (nano::endpoint const & endpoint_a) // Don't keepalive to nodes that already sent us something error |= find_channel (tcp_endpoint) != nullptr; nano::lock_guard lock (mutex); - auto existing (attempts.find (tcp_endpoint)); - error |= existing != attempts.end (); - attempts.insert ({ tcp_endpoint, std::chrono::steady_clock::now () }); + auto inserted (attempts.emplace (tcp_endpoint)); + error |= !inserted.second; } return error; } @@ -450,9 +449,9 @@ void nano::transport::tcp_channels::ongoing_keepalive () void nano::transport::tcp_channels::list (std::deque> & deque_a) { nano::lock_guard lock (mutex); - for (auto i (channels.begin ()), j (channels.end ()); i != j; ++i) + for (auto const & channel : channels.get ()) { - deque_a.push_back (i->channel); + deque_a.push_back (channel.channel); } } diff --git a/nano/node/transport/tcp.hpp b/nano/node/transport/tcp.hpp index ea49908d..a6efe8ce 100644 --- a/nano/node/transport/tcp.hpp +++ b/nano/node/transport/tcp.hpp @@ -137,6 +137,10 @@ namespace transport std::shared_ptr channel; std::shared_ptr socket; std::shared_ptr response_server; + channel_tcp_wrapper (std::shared_ptr const & channel_a, std::shared_ptr const & socket_a, std::shared_ptr const & server_a) : + channel (channel_a), socket (socket_a), response_server (server_a) + { + } nano::tcp_endpoint endpoint () const { return channel->get_tcp_endpoint (); @@ -164,7 +168,12 @@ namespace transport { public: nano::tcp_endpoint endpoint; - std::chrono::steady_clock::time_point last_attempt; + std::chrono::steady_clock::time_point last_attempt{ std::chrono::steady_clock::now () }; + + explicit tcp_endpoint_attempt (nano::tcp_endpoint const & endpoint_a) : + endpoint (endpoint_a) + { + } }; mutable std::mutex mutex; // clang-format off diff --git a/nano/node/transport/udp.cpp b/nano/node/transport/udp.cpp index e9787674..5d542f5b 100644 --- a/nano/node/transport/udp.cpp +++ b/nano/node/transport/udp.cpp @@ -108,7 +108,7 @@ std::shared_ptr nano::transport::udp_channels::ins else { result = std::make_shared (*this, endpoint_a, network_version_a); - channels.get ().insert ({ result }); + channels.get ().insert (result); lock.unlock (); node.network.channel_observer (result); } @@ -598,9 +598,8 @@ bool nano::transport::udp_channels::reachout (nano::endpoint const & endpoint_a) // Don't keepalive to nodes that already sent us something error |= channel (endpoint_l) != nullptr; nano::lock_guard lock (mutex); - auto existing (attempts.find (endpoint_l)); - error |= existing != attempts.end (); - attempts.insert ({ endpoint_l, std::chrono::steady_clock::now () }); + auto inserted (attempts.emplace (endpoint_l)); + error |= !inserted.second; } return error; } @@ -660,9 +659,9 @@ void nano::transport::udp_channels::ongoing_keepalive () void nano::transport::udp_channels::list (std::deque> & deque_a) { nano::lock_guard lock (mutex); - for (auto i (channels.begin ()), j (channels.end ()); i != j; ++i) + for (auto const & channel : channels.get ()) { - deque_a.push_back (i->channel); + deque_a.push_back (channel.channel); } } diff --git a/nano/node/transport/udp.hpp b/nano/node/transport/udp.hpp index 10f81fe6..8acd4b4c 100644 --- a/nano/node/transport/udp.hpp +++ b/nano/node/transport/udp.hpp @@ -118,6 +118,10 @@ namespace transport { public: std::shared_ptr channel; + channel_udp_wrapper (std::shared_ptr const & channel_a) : + channel (channel_a) + { + } nano::endpoint endpoint () const { return channel->get_endpoint (); @@ -143,7 +147,12 @@ namespace transport { public: nano::endpoint endpoint; - std::chrono::steady_clock::time_point last_attempt; + std::chrono::steady_clock::time_point last_attempt{ std::chrono::steady_clock::now () }; + + explicit endpoint_attempt (nano::endpoint const & endpoint_a) : + endpoint (endpoint_a) + { + } }; mutable std::mutex mutex; // clang-format off diff --git a/nano/node/vote_processor.cpp b/nano/node/vote_processor.cpp index b0b6e8bb..6403f8ec 100644 --- a/nano/node/vote_processor.cpp +++ b/nano/node/vote_processor.cpp @@ -142,7 +142,7 @@ void nano::vote_processor::vote (std::shared_ptr vote_a, std::shared } if (process) { - votes.push_back (std::make_pair (vote_a, channel_a)); + votes.emplace_back (vote_a, channel_a); lock.unlock (); condition.notify_all (); @@ -169,7 +169,7 @@ void nano::vote_processor::verify_votes (std::deque verifications; verifications.resize (size); - for (auto & vote : votes_a) + for (auto const & vote : votes_a) { hashes.push_back (vote.first->hash ()); messages.push_back (hashes.back ().bytes.data ()); @@ -180,7 +180,7 @@ void nano::vote_processor::verify_votes (std::deque result; auto i (0); - for (auto & vote : votes_a) + for (auto const & vote : votes_a) { assert (verifications[i] == 1 || verifications[i] == 0); if (verifications[i] == 1) diff --git a/nano/node/wallet.cpp b/nano/node/wallet.cpp index ec4e527d..91aa3d13 100644 --- a/nano/node/wallet.cpp +++ b/nano/node/wallet.cpp @@ -1743,7 +1743,7 @@ void nano::wallets::queue_wallet_action (nano::uint128_t const & amount_a, std:: { { nano::lock_guard action_lock (action_mutex); - actions.insert (std::make_pair (amount_a, std::make_pair (wallet_a, std::move (action_a)))); + actions.emplace (amount_a, std::make_pair (wallet_a, std::move (action_a))); } condition.notify_all (); } diff --git a/nano/node/websocket.cpp b/nano/node/websocket.cpp index b54d9b38..fedb9db2 100644 --- a/nano/node/websocket.cpp +++ b/nano/node/websocket.cpp @@ -430,7 +430,7 @@ void nano::websocket::session::handle_message (boost::property_tree::ptree const } else { - subscriptions.insert (std::make_pair (topic_l, std::move (options_l))); + subscriptions.emplace (topic_l, std::move (options_l)); ws_listener.get_logger ().always_log ("Websocket: new subscription to topic: ", from_topic (topic_l)); ws_listener.increase_subscriber_count (topic_l); } diff --git a/nano/qt_system/entry.cpp b/nano/qt_system/entry.cpp index 92092743..e62356c4 100644 --- a/nano/qt_system/entry.cpp +++ b/nano/qt_system/entry.cpp @@ -28,7 +28,7 @@ int main (int argc, char ** argv) auto wallet (system.nodes[i]->wallets.create (nano::random_wallet_id ())); nano::keypair key; wallet->insert_adhoc (key.prv); - guis.push_back (std::unique_ptr (new nano_qt::wallet (application, processor, *system.nodes[i], wallet, key.pub))); + guis.push_back (std::make_unique (application, processor, *system.nodes[i], wallet, key.pub)); client_tabs->addTab (guis.back ()->client_window, boost::str (boost::format ("Wallet %1%") % i).c_str ()); } client_tabs->show ();