Merge pull request #4632 from clemahieu/priority_sorting_cleanup

Clean up priority scheduler bucket look ups

nano/core_test/scheduler_buckets.cpp

@@ -115,18 +115,6 @@ TEST (buckets, construction)
 	ASSERT_EQ (62, buckets.bucket_count ());
 }
 
-TEST (buckets, index_min)
-{
-	nano::scheduler::buckets buckets;
-	ASSERT_EQ (0, buckets.index (std::numeric_limits<nano::uint128_t>::min ()));
-}
-
-TEST (buckets, index_max)
-{
-	nano::scheduler::buckets buckets;
-	ASSERT_EQ (buckets.bucket_count () - 1, buckets.index (std::numeric_limits<nano::uint128_t>::max ()));
-}
-
 TEST (buckets, insert_Gxrb)
 {
 	nano::scheduler::buckets buckets;

nano/node/scheduler/bucket.cpp

@@ -11,8 +11,9 @@ bool nano::scheduler::bucket::value_type::operator== (value_type const & other_a
 	return time == other_a.time && block->hash () == other_a.block->hash ();
 }
 
-nano::scheduler::bucket::bucket (size_t maximum) :
-	maximum{ maximum }
+nano::scheduler::bucket::bucket (nano::uint128_t minimum_balance, size_t maximum) :
+	maximum{ maximum },
+	minimum_balance{ minimum_balance }
 {
 	debug_assert (maximum > 0);
 }

nano/node/scheduler/bucket.hpp

@@ -27,8 +27,11 @@ class bucket final
 	size_t const maximum;
 
 public:
-	bucket (size_t maximum);
+	bucket (nano::uint128_t minimum_balance, size_t maximum);
 	~bucket ();
+
+	nano::uint128_t const minimum_balance;
+
 	std::shared_ptr<nano::block> top () const;
 	void pop ();
 	void push (uint64_t time, std::shared_ptr<nano::block> block);

nano/node/scheduler/buckets.cpp

@@ -25,21 +25,18 @@ void nano::scheduler::buckets::seek ()
 	}
 }
 
-/**
- * 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.
- */
-nano::scheduler::buckets::buckets (uint64_t maximum) :
-	maximum{ maximum }
+void nano::scheduler::buckets::setup_buckets (uint64_t maximum)
 {
-	auto build_region = [this] (uint128_t const & begin, uint128_t const & end, size_t count) {
+	auto const size_expected = 62;
+	auto bucket_max = std::max<size_t> (1u, maximum / size_expected);
+	auto build_region = [&] (uint128_t const & begin, uint128_t const & end, size_t count) {
 		auto width = (end - begin) / count;
 		for (auto i = 0; i < count; ++i)
 		{
-			minimums.push_back (begin + i * width);
+			buckets_m.push_back (std::make_unique<scheduler::bucket> (begin + i * width, bucket_max));
 		}
 	};
-	minimums.push_back (uint128_t{ 0 });
+	build_region (0, uint128_t{ 1 } << 88, 1);
 	build_region (uint128_t{ 1 } << 88, uint128_t{ 1 } << 92, 2);
 	build_region (uint128_t{ 1 } << 92, uint128_t{ 1 } << 96, 4);
 	build_region (uint128_t{ 1 } << 96, uint128_t{ 1 } << 100, 8);
@@ -48,23 +45,24 @@ nano::scheduler::buckets::buckets (uint64_t maximum) :
 	build_region (uint128_t{ 1 } << 108, uint128_t{ 1 } << 112, 8);
 	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);
-	auto bucket_max = std::max<size_t> (1u, maximum / minimums.size ());
-	for (size_t i = 0u, n = minimums.size (); i < n; ++i)
-	{
-		buckets_m.push_back (std::make_unique<scheduler::bucket> (bucket_max));
-	}
-	current = buckets_m.begin ();
+	build_region (uint128_t{ 1 } << 120, uint128_t{ 1 } << 127, 1);
+
+	debug_assert (buckets_m.size () == size_expected);
 }
 
-nano::scheduler::buckets::~buckets ()
+/**
+ * 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.
+ */
+nano::scheduler::buckets::buckets (uint64_t maximum) :
+	maximum{ maximum }
 {
+	setup_buckets (maximum);
+	current = buckets_m.begin ();
 }
 
-std::size_t nano::scheduler::buckets::index (nano::uint128_t const & balance) const
+nano::scheduler::buckets::~buckets ()
 {
-	auto index = std::upper_bound (minimums.begin (), minimums.end (), balance) - minimums.begin () - 1;
-	return index;
 }
 
 /**
@@ -74,8 +72,8 @@ std::size_t nano::scheduler::buckets::index (nano::uint128_t const & balance) co
 void nano::scheduler::buckets::push (uint64_t time, std::shared_ptr<nano::block> block, nano::amount const & priority)
 {
 	auto was_empty = empty ();
-	auto & bucket = buckets_m[index (priority.number ())];
-	bucket->push (time, block);
+	auto & bucket = find_bucket (priority.number ());
+	bucket.push (time, block);
 	if (was_empty)
 	{
 		seek ();
@@ -138,6 +136,15 @@ void nano::scheduler::buckets::dump () const
 	std::cerr << "current: " << current - buckets_m.begin () << '\n';
 }
 
+auto nano::scheduler::buckets::find_bucket (nano::uint128_t priority) -> bucket &
+{
+	auto it = std::upper_bound (buckets_m.begin (), buckets_m.end (), priority, [] (nano::uint128_t const & priority, std::unique_ptr<bucket> const & bucket) {
+		return priority < bucket->minimum_balance;
+	});
+	release_assert (it != buckets_m.begin ()); // There should always be a bucket with a minimum_balance of 0
+	return **std::prev (it);
+}
+
 std::unique_ptr<nano::container_info_component> nano::scheduler::buckets::collect_container_info (std::string const & name)
 {
 	auto composite = std::make_unique<container_info_composite> (name);

nano/node/scheduler/buckets.hpp

@@ -27,11 +27,7 @@ class bucket;
 class buckets final
 {
 	/** container for the buckets to be read in round robin fashion */
-	std::deque<std::unique_ptr<bucket>> 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::deque<nano::uint128_t> minimums;
+	std::vector<std::unique_ptr<bucket>> buckets_m;
 
 	/** index of bucket to read next */
 	decltype (buckets_m)::const_iterator current;
@@ -41,6 +37,7 @@ class buckets final
 
 	void next ();
 	void seek ();
+	void setup_buckets (uint64_t maximum);
 
 public:
 	buckets (uint64_t maximum = 250000u);
@@ -53,7 +50,7 @@ class buckets final
 	std::size_t bucket_size (std::size_t index) const;
 	bool empty () const;
 	void dump () const;
-	std::size_t index (nano::uint128_t const & balance) const;
+	bucket & find_bucket (nano::uint128_t priority);
 
 	std::unique_ptr<nano::container_info_component> collect_container_info (std::string const &);
 };