Skip to content

Commit

Permalink
Fix performance issue in block_uniquer cleanup. (#4326)
Browse files Browse the repository at this point in the history
The entire container is now cleaned periodically instead of the previous, more complex way.
  • Loading branch information
clemahieu authored Nov 5, 2023
1 parent 5bb885a commit 6244918
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 21 deletions.
15 changes: 7 additions & 8 deletions nano/core_test/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include <boost/property_tree/json_parser.hpp>

#include <thread>

#include <crypto/ed25519-donna/ed25519.h>

TEST (ed25519, signing)
Expand Down Expand Up @@ -589,17 +591,14 @@ TEST (block_uniquer, cleanup)
.build_shared ();

nano::block_uniquer uniquer;
auto block3 (uniquer.unique (block1));
auto block4 (uniquer.unique (block2));
auto block3 = uniquer.unique (block1);
auto block4 = uniquer.unique (block2);
block2.reset ();
block4.reset ();
ASSERT_EQ (2, uniquer.size ());
auto iterations (0);
while (uniquer.size () == 2)
{
auto block5 (uniquer.unique (block1));
ASSERT_LT (iterations++, 200);
}
std::this_thread::sleep_for (nano::block_uniquer::cleanup_cutoff);
auto block5 = uniquer.unique (block1);
ASSERT_EQ (1, uniquer.size ());
}

TEST (block_builder, from)
Expand Down
19 changes: 7 additions & 12 deletions nano/lib/blocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1881,24 +1881,19 @@ std::shared_ptr<nano::block> nano::block_uniquer::unique (std::shared_ptr<nano::
{
existing = block_a;
}
release_assert (std::numeric_limits<CryptoPP::word32>::max () > blocks.size ());
for (auto i (0); i < cleanup_count && !blocks.empty (); ++i)
auto now = std::chrono::steady_clock::now ();
if (cleanup_cutoff < now - cleanup_last)
{
auto random_offset (nano::random_pool::generate_word32 (0, static_cast<CryptoPP::word32> (blocks.size () - 1)));
auto existing (std::next (blocks.begin (), random_offset));
if (existing == blocks.end ())
cleanup_last = now;
for (auto i = blocks.begin (), n = blocks.end (); i != n;)
{
existing = blocks.begin ();
}
if (existing != blocks.end ())
{
if (auto block_l = existing->second.lock ())
if (auto block_l = i->second.lock ())
{
// Still live
++i;
}
else
{
blocks.erase (existing);
i = blocks.erase (i);
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion nano/lib/blocks.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,10 @@ class block_uniquer
private:
nano::mutex mutex{ mutex_identifier (mutexes::block_uniquer) };
std::unordered_map<std::remove_const_t<value_type::first_type>, value_type::second_type> blocks;
static unsigned constexpr cleanup_count = 2;
std::chrono::steady_clock::time_point cleanup_last{ std::chrono::steady_clock::now () };

public:
static std::chrono::milliseconds constexpr cleanup_cutoff{ 500 };
};

std::unique_ptr<container_info_component> collect_container_info (block_uniquer & block_uniquer, std::string const & name);
Expand Down

0 comments on commit 6244918

Please sign in to comment.