From ef3167a2360cfa0fb61da778c6d1bc586e63db19 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 8 Sep 2023 10:35:43 +0100 Subject: [PATCH 1/4] Forward declaring scheduler::buckets so it doesn't need to be included directly in the priority header. --- nano/node/scheduler/priority.cpp | 20 +++++++++++--------- nano/node/scheduler/priority.hpp | 4 ++-- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/nano/node/scheduler/priority.cpp b/nano/node/scheduler/priority.cpp index 0499fd2a28..0ee92f4643 100644 --- a/nano/node/scheduler/priority.cpp +++ b/nano/node/scheduler/priority.cpp @@ -1,9 +1,11 @@ #include +#include #include nano::scheduler::priority::priority (nano::node & node_a, nano::stats & stats_a) : node{ node_a }, - stats{ stats_a } + stats{ stats_a }, + buckets{ std::make_unique () } { } @@ -60,7 +62,7 @@ bool nano::scheduler::priority::activate (nano::account const & account_a, nano: auto balance = node.ledger.balance (transaction, hash); auto previous_balance = node.ledger.balance (transaction, conf_info.frontier); nano::lock_guard lock{ mutex }; - buckets.push (info->modified, block, std::max (balance, previous_balance)); + buckets->push (info->modified, block, std::max (balance, previous_balance)); notify (); return true; // Activated } @@ -77,12 +79,12 @@ void nano::scheduler::priority::notify () std::size_t nano::scheduler::priority::size () const { nano::lock_guard lock{ mutex }; - return buckets.size () + manual_queue.size (); + return buckets->size () + manual_queue.size (); } bool nano::scheduler::priority::empty_locked () const { - return buckets.empty () && manual_queue.empty (); + return buckets->empty () && manual_queue.empty (); } bool nano::scheduler::priority::empty () const @@ -93,12 +95,12 @@ bool nano::scheduler::priority::empty () const std::size_t nano::scheduler::priority::priority_queue_size () const { - return buckets.size (); + return buckets->size (); } bool nano::scheduler::priority::priority_queue_predicate () const { - return node.active.vacancy () > 0 && !buckets.empty (); + return node.active.vacancy () > 0 && !buckets->empty (); } bool nano::scheduler::priority::manual_queue_predicate () const @@ -133,8 +135,8 @@ void nano::scheduler::priority::run () } else if (priority_queue_predicate ()) { - auto block = buckets.top (); - buckets.pop (); + auto block = buckets->top (); + buckets->pop (); lock.unlock (); stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::insert_priority); auto result = node.active.insert (block); @@ -163,6 +165,6 @@ std::unique_ptr nano::scheduler::priority::colle auto composite = std::make_unique (name); composite->add_component (std::make_unique (container_info{ "manual_queue", manual_queue.size (), sizeof (decltype (manual_queue)::value_type) })); - composite->add_component (buckets.collect_container_info ("buckets")); + composite->add_component (buckets->collect_container_info ("buckets")); return composite; } diff --git a/nano/node/scheduler/priority.hpp b/nano/node/scheduler/priority.hpp index a1d825d3f4..1358286713 100644 --- a/nano/node/scheduler/priority.hpp +++ b/nano/node/scheduler/priority.hpp @@ -2,7 +2,6 @@ #include #include -#include #include @@ -19,6 +18,7 @@ class node; namespace nano::scheduler { +class buckets; class priority final { public: @@ -52,7 +52,7 @@ class priority final bool priority_queue_predicate () const; bool manual_queue_predicate () const; - nano::scheduler::buckets buckets; + std::unique_ptr buckets; std::deque, boost::optional, nano::election_behavior>> manual_queue; bool stopped{ false }; From 6d47d24f73ec0b68a500176a0271ba5999dcb5e2 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 8 Sep 2023 10:41:14 +0100 Subject: [PATCH 2/4] Replacing vectors with deques. --- nano/node/scheduler/buckets.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nano/node/scheduler/buckets.hpp b/nano/node/scheduler/buckets.hpp index 0f588fb9c4..5b6cc9e43f 100644 --- a/nano/node/scheduler/buckets.hpp +++ b/nano/node/scheduler/buckets.hpp @@ -4,7 +4,7 @@ #include #include -#include +#include namespace nano { @@ -36,14 +36,14 @@ class buckets final using priority = std::set; /** container for the buckets to be read in round robin fashion */ - std::vector buckets_m; + std::deque buckets_m; /** thresholds that define the bands for each bucket, the minimum balance an account must have to enter a bucket, * the container writes a block to the lowest indexed bucket that has balance larger than the bucket's minimum value */ - std::vector minimums; + std::deque minimums; /** Contains bucket indicies to iterate over when making the next scheduling decision */ - std::vector schedule; + std::deque schedule; /** index of bucket to read next */ decltype (schedule)::const_iterator current; From 32b151b4156c0f8585c9fbb810eb869d47e46383 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 8 Sep 2023 12:16:00 +0100 Subject: [PATCH 3/4] Split nano::scheduler::bucket in to its own class and file --- nano/node/CMakeLists.txt | 2 ++ nano/node/scheduler/bucket.cpp | 62 +++++++++++++++++++++++++++++++++ nano/node/scheduler/bucket.hpp | 39 +++++++++++++++++++++ nano/node/scheduler/buckets.cpp | 52 +++++++++++---------------- nano/node/scheduler/buckets.hpp | 18 +++------- 5 files changed, 129 insertions(+), 44 deletions(-) create mode 100644 nano/node/scheduler/bucket.cpp create mode 100644 nano/node/scheduler/bucket.hpp diff --git a/nano/node/CMakeLists.txt b/nano/node/CMakeLists.txt index df13b9208b..ccb4d4fe76 100644 --- a/nano/node/CMakeLists.txt +++ b/nano/node/CMakeLists.txt @@ -194,6 +194,8 @@ add_library( rocksdb/rocksdb_iterator.hpp rocksdb/rocksdb_txn.hpp rocksdb/rocksdb_txn.cpp + scheduler/bucket.cpp + scheduler/bucket.hpp scheduler/buckets.cpp scheduler/buckets.hpp scheduler/component.hpp diff --git a/nano/node/scheduler/bucket.cpp b/nano/node/scheduler/bucket.cpp new file mode 100644 index 0000000000..51ba1626a5 --- /dev/null +++ b/nano/node/scheduler/bucket.cpp @@ -0,0 +1,62 @@ +#include +#include + +bool nano::scheduler::bucket::value_type::operator< (value_type const & other_a) const +{ + return time < other_a.time || (time == other_a.time && block->hash () < other_a.block->hash ()); +} + +bool nano::scheduler::bucket::value_type::operator== (value_type const & other_a) const +{ + return time == other_a.time && block->hash () == other_a.block->hash (); +} + +nano::scheduler::bucket::bucket (size_t maximum) : + maximum{ maximum } +{ + debug_assert (maximum > 0); +} + +nano::scheduler::bucket::~bucket () +{ +} + +std::shared_ptr nano::scheduler::bucket::top () const +{ + debug_assert (!queue.empty ()); + return queue.begin ()->block; +} + +void nano::scheduler::bucket::pop () +{ + debug_assert (!queue.empty ()); + queue.erase (queue.begin ()); +} + +void nano::scheduler::bucket::push (uint64_t time, std::shared_ptr block) +{ + queue.insert ({ time, block }); + if (queue.size () > maximum) + { + debug_assert (!queue.empty ()); + queue.erase (--queue.end ()); + } +} + +size_t nano::scheduler::bucket::size () const +{ + return queue.size (); +} + +bool nano::scheduler::bucket::empty () const +{ + return queue.empty (); +} + +void nano::scheduler::bucket::dump () const +{ + for (auto const & item : queue) + { + std::cerr << item.time << ' ' << item.block->hash ().to_string () << '\n'; + } +} diff --git a/nano/node/scheduler/bucket.hpp b/nano/node/scheduler/bucket.hpp new file mode 100644 index 0000000000..2f32c17d59 --- /dev/null +++ b/nano/node/scheduler/bucket.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include +#include + +namespace nano +{ +class block; +} +namespace nano::scheduler +{ +/** A class which holds an ordered set of blocks to be scheduled, ordered by their block arrival time + */ +class bucket final +{ + class value_type + { + public: + uint64_t time; + std::shared_ptr block; + bool operator< (value_type const & other_a) const; + bool operator== (value_type const & other_a) const; + }; + std::set queue; + size_t const maximum; + +public: + bucket (size_t maximum); + ~bucket (); + std::shared_ptr top () const; + void pop (); + void push (uint64_t time, std::shared_ptr block); + size_t size () const; + bool empty () const; + void dump () const; +}; +} // namespace nano::scheduler diff --git a/nano/node/scheduler/buckets.cpp b/nano/node/scheduler/buckets.cpp index c01bbbd77b..59344f7b72 100644 --- a/nano/node/scheduler/buckets.cpp +++ b/nano/node/scheduler/buckets.cpp @@ -1,19 +1,10 @@ #include #include +#include #include #include -bool nano::scheduler::buckets::value_type::operator< (value_type const & other_a) const -{ - return time < other_a.time || (time == other_a.time && block->hash () < other_a.block->hash ()); -} - -bool nano::scheduler::buckets::value_type::operator== (value_type const & other_a) const -{ - return time == other_a.time && block->hash () == other_a.block->hash (); -} - /** Moves the bucket pointer to the next bucket */ void nano::scheduler::buckets::next () { @@ -28,7 +19,7 @@ void nano::scheduler::buckets::next () void nano::scheduler::buckets::seek () { next (); - for (std::size_t i = 0, n = schedule.size (); buckets_m[*current].empty () && i < n; ++i) + for (std::size_t i = 0, n = schedule.size (); buckets_m[*current]->empty () && i < n; ++i) { next (); } @@ -67,11 +58,19 @@ nano::scheduler::buckets::buckets (uint64_t maximum) : build_region (uint128_t{ 1 } << 112, uint128_t{ 1 } << 116, 4); build_region (uint128_t{ 1 } << 116, uint128_t{ 1 } << 120, 2); minimums.push_back (uint128_t{ 1 } << 120); - buckets_m.resize (minimums.size ()); + auto bucket_max = std::max (1u, maximum / minimums.size ()); + for (size_t i = 0u, n = minimums.size (); i < n; ++i) + { + buckets_m.push_back (std::make_unique (bucket_max)); + } populate_schedule (); current = schedule.begin (); } +nano::scheduler::buckets::~buckets () +{ +} + std::size_t nano::scheduler::buckets::index (nano::uint128_t const & balance) const { auto index = std::upper_bound (minimums.begin (), minimums.end (), balance) - minimums.begin () - 1; @@ -86,11 +85,7 @@ void nano::scheduler::buckets::push (uint64_t time, std::shared_ptr { auto was_empty = empty (); auto & bucket = buckets_m[index (priority.number ())]; - bucket.emplace (value_type{ time, block }); - if (bucket.size () > std::max (decltype (maximum){ 1 }, maximum / buckets_m.size ())) - { - bucket.erase (--bucket.end ()); - } + bucket->push (time, block); if (was_empty) { seek (); @@ -101,8 +96,7 @@ void nano::scheduler::buckets::push (uint64_t time, std::shared_ptr std::shared_ptr nano::scheduler::buckets::top () const { debug_assert (!empty ()); - debug_assert (!buckets_m[*current].empty ()); - auto result = buckets_m[*current].begin ()->block; + auto result = buckets_m[*current]->top (); return result; } @@ -110,9 +104,8 @@ std::shared_ptr nano::scheduler::buckets::top () const void nano::scheduler::buckets::pop () { debug_assert (!empty ()); - debug_assert (!buckets_m[*current].empty ()); auto & bucket = buckets_m[*current]; - bucket.erase (bucket.begin ()); + bucket->pop (); seek (); } @@ -120,9 +113,9 @@ void nano::scheduler::buckets::pop () std::size_t nano::scheduler::buckets::size () const { std::size_t result{ 0 }; - for (auto const & queue : buckets_m) + for (auto const & bucket : buckets_m) { - result += queue.size (); + result += bucket->size (); } return result; } @@ -136,24 +129,21 @@ std::size_t nano::scheduler::buckets::bucket_count () const /** Returns number of items in bucket with index 'index' */ std::size_t nano::scheduler::buckets::bucket_size (std::size_t index) const { - return buckets_m[index].size (); + return buckets_m[index]->size (); } /** Returns true if all buckets are empty */ bool nano::scheduler::buckets::empty () const { - return std::all_of (buckets_m.begin (), buckets_m.end (), [] (priority const & bucket_a) { return bucket_a.empty (); }); + return std::all_of (buckets_m.begin (), buckets_m.end (), [] (auto const & bucket) { return bucket->empty (); }); } /** Print the state of the class in stderr */ void nano::scheduler::buckets::dump () const { - for (auto const & i : buckets_m) + for (auto const & bucket : buckets_m) { - for (auto const & j : i) - { - std::cerr << j.time << ' ' << j.block->hash ().to_string () << '\n'; - } + bucket->dump (); } std::cerr << "current: " << std::to_string (*current) << '\n'; } @@ -164,7 +154,7 @@ std::unique_ptr nano::scheduler::buckets::collec for (auto i = 0; i < buckets_m.size (); ++i) { auto const & bucket = buckets_m[i]; - composite->add_component (std::make_unique (container_info{ std::to_string (i), bucket.size (), 0 })); + composite->add_component (std::make_unique (container_info{ std::to_string (i), bucket->size (), 0 })); } return composite; } diff --git a/nano/node/scheduler/buckets.hpp b/nano/node/scheduler/buckets.hpp index 5b6cc9e43f..0c4b745ac8 100644 --- a/nano/node/scheduler/buckets.hpp +++ b/nano/node/scheduler/buckets.hpp @@ -3,8 +3,9 @@ #include #include -#include +#include #include +#include namespace nano { @@ -12,6 +13,7 @@ class block; } namespace nano::scheduler { +class bucket; /** A container for holding blocks and their arrival/creation time. * * The container consists of a number of buckets. Each bucket holds an ordered set of 'value_type' items. @@ -24,19 +26,8 @@ namespace nano::scheduler */ class buckets final { - class value_type - { - public: - uint64_t time; - std::shared_ptr block; - bool operator< (value_type const & other_a) const; - bool operator== (value_type const & other_a) const; - }; - - using priority = std::set; - /** container for the buckets to be read in round robin fashion */ - std::deque buckets_m; + std::deque> buckets_m; /** thresholds that define the bands for each bucket, the minimum balance an account must have to enter a bucket, * the container writes a block to the lowest indexed bucket that has balance larger than the bucket's minimum value */ @@ -57,6 +48,7 @@ class buckets final public: buckets (uint64_t maximum = 250000u); + ~buckets (); void push (uint64_t time, std::shared_ptr block, nano::amount const & priority); std::shared_ptr top () const; void pop (); From 50648b27ee71c4b924c27c368e070d14d8034f99 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 8 Sep 2023 12:27:57 +0100 Subject: [PATCH 4/4] Removing buckets::schedule as it's confusing and unnecessary. --- nano/node/scheduler/buckets.cpp | 24 +++++++----------------- nano/node/scheduler/buckets.hpp | 6 +----- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/nano/node/scheduler/buckets.cpp b/nano/node/scheduler/buckets.cpp index 59344f7b72..c033f6f6ca 100644 --- a/nano/node/scheduler/buckets.cpp +++ b/nano/node/scheduler/buckets.cpp @@ -9,9 +9,9 @@ void nano::scheduler::buckets::next () { ++current; - if (current == schedule.end ()) + if (current == buckets_m.end ()) { - current = schedule.begin (); + current = buckets_m.begin (); } } @@ -19,21 +19,12 @@ void nano::scheduler::buckets::next () void nano::scheduler::buckets::seek () { next (); - for (std::size_t i = 0, n = schedule.size (); buckets_m[*current]->empty () && i < n; ++i) + for (std::size_t i = 0, n = buckets_m.size (); (*current)->empty () && i < n; ++i) { next (); } } -/** Initialise the schedule vector */ -void nano::scheduler::buckets::populate_schedule () -{ - for (auto i = 0; i < buckets_m.size (); ++i) - { - schedule.push_back (i); - } -} - /** * Prioritization constructor, construct a container containing approximately 'maximum' number of blocks. * @param maximum number of blocks that this container can hold, this is a soft and approximate limit. @@ -63,8 +54,7 @@ nano::scheduler::buckets::buckets (uint64_t maximum) : { buckets_m.push_back (std::make_unique (bucket_max)); } - populate_schedule (); - current = schedule.begin (); + current = buckets_m.begin (); } nano::scheduler::buckets::~buckets () @@ -96,7 +86,7 @@ void nano::scheduler::buckets::push (uint64_t time, std::shared_ptr std::shared_ptr nano::scheduler::buckets::top () const { debug_assert (!empty ()); - auto result = buckets_m[*current]->top (); + auto result = (*current)->top (); return result; } @@ -104,7 +94,7 @@ std::shared_ptr nano::scheduler::buckets::top () const void nano::scheduler::buckets::pop () { debug_assert (!empty ()); - auto & bucket = buckets_m[*current]; + auto & bucket = *current; bucket->pop (); seek (); } @@ -145,7 +135,7 @@ void nano::scheduler::buckets::dump () const { bucket->dump (); } - std::cerr << "current: " << std::to_string (*current) << '\n'; + std::cerr << "current: " << current - buckets_m.begin () << '\n'; } std::unique_ptr nano::scheduler::buckets::collect_container_info (std::string const & name) diff --git a/nano/node/scheduler/buckets.hpp b/nano/node/scheduler/buckets.hpp index 0c4b745ac8..967b4408f7 100644 --- a/nano/node/scheduler/buckets.hpp +++ b/nano/node/scheduler/buckets.hpp @@ -33,18 +33,14 @@ class buckets final * the container writes a block to the lowest indexed bucket that has balance larger than the bucket's minimum value */ std::deque minimums; - /** Contains bucket indicies to iterate over when making the next scheduling decision */ - std::deque schedule; - /** index of bucket to read next */ - decltype (schedule)::const_iterator current; + decltype (buckets_m)::const_iterator current; /** maximum number of blocks in whole container, each bucket's maximum is maximum / bucket_number */ uint64_t const maximum; void next (); void seek (); - void populate_schedule (); public: buckets (uint64_t maximum = 250000u);