Merge pull request #4759 from clemahieu/store_iterator_cleanup

Convert store iterators to variants rather than using polymorphism.

nano/core_test/block_store.cpp

@@ -798,7 +798,7 @@ TEST (block_store, large_iteration)
 	// Reverse iteration
 	std::unordered_set<nano::account> accounts3;
 	previous = std::numeric_limits<nano::uint256_t>::max ();
-	for (auto i (store->account.rbegin (transaction)), n (store->account.end (transaction)); i != n; --i)
+	for (auto i (store->account.rbegin (transaction)), n (store->account.rend (transaction)); i != n; ++i)
 	{
 		nano::account current (i->first);
 		ASSERT_LT (current.number (), previous.number ());
@@ -1254,7 +1254,7 @@ TEST (block_store, online_weight)
 		auto transaction (store->tx_begin_write ());
 		ASSERT_EQ (0, store->online_weight.count (transaction));
 		ASSERT_EQ (store->online_weight.end (transaction), store->online_weight.begin (transaction));
-		ASSERT_EQ (store->online_weight.end (transaction), store->online_weight.rbegin (transaction));
+		ASSERT_EQ (store->online_weight.rend (transaction), store->online_weight.rbegin (transaction));
 		store->online_weight.put (transaction, 1, 2);
 		store->online_weight.put (transaction, 3, 4);
 	}
@@ -1266,18 +1266,18 @@ TEST (block_store, online_weight)
 		ASSERT_EQ (1, item->first);
 		ASSERT_EQ (2, item->second.number ());
 		auto item_last (store->online_weight.rbegin (transaction));
-		ASSERT_NE (store->online_weight.end (transaction), item_last);
+		ASSERT_NE (store->online_weight.rend (transaction), item_last);
 		ASSERT_EQ (3, item_last->first);
 		ASSERT_EQ (4, item_last->second.number ());
 		store->online_weight.del (transaction, 1);
 		ASSERT_EQ (1, store->online_weight.count (transaction));
-		ASSERT_EQ (store->online_weight.begin (transaction), store->online_weight.rbegin (transaction));
+		ASSERT_EQ (*store->online_weight.begin (transaction), *store->online_weight.rbegin (transaction));
 		store->online_weight.del (transaction, 3);
 	}
 	auto transaction (store->tx_begin_read ());
 	ASSERT_EQ (0, store->online_weight.count (transaction));
 	ASSERT_EQ (store->online_weight.end (transaction), store->online_weight.begin (transaction));
-	ASSERT_EQ (store->online_weight.end (transaction), store->online_weight.rbegin (transaction));
+	ASSERT_EQ (store->online_weight.rend (transaction), store->online_weight.rbegin (transaction));
 }
 
 TEST (block_store, pruned_blocks)

nano/node/node.cpp

@@ -755,7 +755,7 @@ void nano::node::long_inactivity_cleanup ()
 	if (store.online_weight.count (transaction) > 0)
 	{
 		auto sample (store.online_weight.rbegin (transaction));
-		auto n (store.online_weight.end (transaction));
+		auto n (store.online_weight.rend (transaction));
 		debug_assert (sample != n);
 		auto const one_week_ago = static_cast<std::size_t> ((std::chrono::system_clock::now () - std::chrono::hours (7 * 24)).time_since_epoch ().count ());
 		perform_cleanup = sample->first < one_week_ago;

nano/node/wallet.cpp

@@ -10,6 +10,7 @@
 #include <nano/secure/ledger_set_any.hpp>
 #include <nano/secure/ledger_set_confirmed.hpp>
 #include <nano/store/lmdb/iterator.hpp>
+#include <nano/store/typed_iterator_templ.hpp>
 
 #include <boost/format.hpp>
 #include <boost/polymorphic_cast.hpp>
@@ -19,6 +20,8 @@
 
 #include <argon2.h>
 
+template class nano::store::typed_iterator<nano::account, nano::wallet_value>;
+
 nano::uint256_union nano::wallet_store::check (store::transaction const & transaction_a)
 {
 	nano::wallet_value value (entry_get_raw (transaction_a, nano::wallet_store::check_special));
@@ -548,7 +551,8 @@ bool nano::wallet_store::exists (store::transaction const & transaction_a, nano:
 void nano::wallet_store::serialize_json (store::transaction const & transaction_a, std::string & string_a)
 {
 	boost::property_tree::ptree tree;
-	for (store::iterator<nano::uint256_union, nano::wallet_value> i (std::make_unique<nano::store::lmdb::iterator<nano::uint256_union, nano::wallet_value>> (transaction_a, env, handle)), n (nullptr); i != n; ++i)
+	using iterator = store::typed_iterator<nano::uint256_union, nano::wallet_value>;
+	for (iterator i{ store::iterator{ store::lmdb::iterator::begin (env.tx (transaction_a), handle) } }, n{ store::iterator{ store::lmdb::iterator::end (env.tx (transaction_a), handle) } }; i != n; ++i)
 	{
 		tree.put (i->first.to_string (), i->second.key.to_string ());
 	}
@@ -1359,13 +1363,15 @@ nano::wallets::wallets (bool error_a, nano::node & node_a) :
 		status |= mdb_dbi_open (env.tx (transaction), "send_action_ids", MDB_CREATE, &send_action_ids);
 		release_assert (status == 0);
 		std::string beginning (nano::uint256_union (0).to_string ());
+		nano::store::lmdb::db_val beginning_val{ beginning.size (), const_cast<char *> (beginning.c_str ()) };
 		std::string end ((nano::uint256_union (nano::uint256_t (0) - nano::uint256_t (1))).to_string ());
-		store::iterator<std::array<char, 64>, nano::no_value> i (std::make_unique<nano::store::lmdb::iterator<std::array<char, 64>, nano::no_value>> (transaction, env, handle, nano::store::lmdb::db_val (beginning.size (), const_cast<char *> (beginning.c_str ()))));
-		store::iterator<std::array<char, 64>, nano::no_value> n (std::make_unique<nano::store::lmdb::iterator<std::array<char, 64>, nano::no_value>> (transaction, env, handle, nano::store::lmdb::db_val (end.size (), const_cast<char *> (end.c_str ()))));
+		nano::store::lmdb::db_val end_val{ end.size (), const_cast<char *> (end.c_str ()) };
+		store::iterator i{ store::lmdb::iterator::lower_bound (env.tx (transaction), handle, beginning_val) };
+		store::iterator n{ store::lmdb::iterator::lower_bound (env.tx (transaction), handle, end_val) };
 		for (; i != n; ++i)
 		{
 			nano::wallet_id id;
-			std::string text (i->first.data (), i->first.size ());
+			std::string text (reinterpret_cast<char const *> (i->first.data ()), i->first.size ());
 			auto error (id.decode_hex (text));
 			release_assert (!error);
 			release_assert (items.find (id) == items.end ());
@@ -1488,13 +1494,15 @@ void nano::wallets::reload ()
 	auto transaction (tx_begin_write ());
 	std::unordered_set<nano::uint256_union> stored_items;
 	std::string beginning (nano::uint256_union (0).to_string ());
+	nano::store::lmdb::db_val beginning_val{ beginning.size (), const_cast<char *> (beginning.c_str ()) };
 	std::string end ((nano::uint256_union (nano::uint256_t (0) - nano::uint256_t (1))).to_string ());
-	store::iterator<std::array<char, 64>, nano::no_value> i (std::make_unique<nano::store::lmdb::iterator<std::array<char, 64>, nano::no_value>> (transaction, env, handle, nano::store::lmdb::db_val (beginning.size (), const_cast<char *> (beginning.c_str ()))));
-	store::iterator<std::array<char, 64>, nano::no_value> n (std::make_unique<nano::store::lmdb::iterator<std::array<char, 64>, nano::no_value>> (transaction, env, handle, nano::store::lmdb::db_val (end.size (), const_cast<char *> (end.c_str ()))));
+	nano::store::lmdb::db_val end_val{ end.size (), const_cast<char *> (end.c_str ()) };
+	store::iterator i{ store::lmdb::iterator::lower_bound (env.tx (transaction), handle, beginning_val) };
+	store::iterator n{ store::lmdb::iterator::lower_bound (env.tx (transaction), handle, end_val) };
 	for (; i != n; ++i)
 	{
 		nano::wallet_id id;
-		std::string text (i->first.data (), i->first.size ());
+		std::string text (reinterpret_cast<char const *> (i->first.data ()), i->first.size ());
 		auto error (id.decode_hex (text));
 		debug_assert (!error);
 		// New wallet
@@ -1755,20 +1763,22 @@ nano::uint128_t const nano::wallets::high_priority = std::numeric_limits<nano::u
 
 auto nano::wallet_store::begin (store::transaction const & transaction_a) -> iterator
 {
-	iterator result (std::make_unique<nano::store::lmdb::iterator<nano::account, nano::wallet_value>> (transaction_a, env, handle, nano::store::lmdb::db_val (nano::account (special_count))));
-	return result;
+	nano::account account{ special_count };
+	nano::store::lmdb::db_val val{ account };
+	return iterator{ store::iterator{ store::lmdb::iterator::lower_bound (env.tx (transaction_a), handle, val) } };
 }
 
 auto nano::wallet_store::begin (store::transaction const & transaction_a, nano::account const & key) -> iterator
 {
-	iterator result (std::make_unique<nano::store::lmdb::iterator<nano::account, nano::wallet_value>> (transaction_a, env, handle, nano::store::lmdb::db_val (key)));
-	return result;
+	nano::account account (key);
+	nano::store::lmdb::db_val val{ account };
+	return iterator{ store::iterator{ store::lmdb::iterator::lower_bound (env.tx (transaction_a), handle, val) } };
 }
 
 auto nano::wallet_store::find (store::transaction const & transaction_a, nano::account const & key) -> iterator
 {
-	auto result (begin (transaction_a, key));
-	iterator end{ nullptr };
+	auto result = begin (transaction_a, key);
+	auto end = this->end (transaction_a);
 	if (result != end)
 	{
 		if (result->first == key)
@@ -1789,7 +1799,7 @@ auto nano::wallet_store::find (store::transaction const & transaction_a, nano::a
 
 auto nano::wallet_store::end (store::transaction const & transaction_a) -> iterator
 {
-	return iterator{ nullptr };
+	return iterator{ store::iterator{ store::lmdb::iterator::end (env.tx (transaction_a), handle) } };
 }
 
 nano::mdb_wallets_store::mdb_wallets_store (std::filesystem::path const & path_a, nano::lmdb_config const & lmdb_config_a) :

nano/node/wallet.hpp

@@ -9,6 +9,7 @@
 #include <nano/store/component.hpp>
 #include <nano/store/lmdb/lmdb.hpp>
 #include <nano/store/lmdb/wallet_value.hpp>
+#include <nano/store/typed_iterator.hpp>
 
 #include <atomic>
 #include <mutex>
@@ -58,7 +59,7 @@ enum class key_type
 class wallet_store final
 {
 public:
-	using iterator = store::iterator<nano::account, nano::wallet_value>;
+	using iterator = store::typed_iterator<nano::account, nano::wallet_value>;
 
 public:
 	wallet_store (bool &, nano::kdf &, store::transaction &, store::lmdb::env &, nano::account, unsigned, std::string const &);

nano/store/CMakeLists.txt

@@ -8,7 +8,6 @@ add_library(
   db_val.hpp
   db_val_impl.hpp
   iterator.hpp
-  iterator_impl.hpp
   final_vote.hpp
   lmdb/account.hpp
   lmdb/block.hpp
@@ -33,6 +32,8 @@ add_library(
   pending.hpp
   pruned.hpp
   rep_weight.hpp
+  reverse_iterator.hpp
+  reverse_iterator_templ.hpp
   rocksdb/account.hpp
   rocksdb/block.hpp
   rocksdb/confirmation_height.hpp
@@ -47,9 +48,12 @@ add_library(
   rocksdb/rocksdb.hpp
   rocksdb/iterator.hpp
   rocksdb/transaction_impl.hpp
+  rocksdb/utility.hpp
   rocksdb/version.hpp
   tables.hpp
   transaction.hpp
+  typed_iterator.hpp
+  typed_iterator_templ.hpp
   version.hpp
   versioning.hpp
   account.cpp
@@ -58,13 +62,13 @@ add_library(
   confirmation_height.cpp
   db_val.cpp
   iterator.cpp
-  iterator_impl.cpp
   final_vote.cpp
   lmdb/account.cpp
   lmdb/block.cpp
   lmdb/confirmation_height.cpp
   lmdb/db_val.cpp
   lmdb/final_vote.cpp
+  lmdb/iterator.cpp
   lmdb/lmdb.cpp
   lmdb/lmdb_env.cpp
   lmdb/transaction.cpp
@@ -79,20 +83,24 @@ add_library(
   peer.cpp
   pending.cpp
   pruned.cpp
+  rep_weight.cpp
   rocksdb/account.cpp
   rocksdb/block.cpp
   rocksdb/confirmation_height.cpp
   rocksdb/db_val.cpp
   rocksdb/final_vote.cpp
+  rocksdb/iterator.cpp
   rocksdb/online_weight.cpp
   rocksdb/peer.cpp
   rocksdb/pending.cpp
   rocksdb/pruned.cpp
   rocksdb/rep_weight.cpp
   rocksdb/rocksdb.cpp
   rocksdb/transaction.cpp
+  rocksdb/utility.cpp
   rocksdb/version.cpp
   transaction.cpp
+  typed_iterator.cpp
   version.cpp
   versioning.cpp
   write_queue.hpp

nano/store/account.cpp

@@ -1,4 +1,9 @@
 #include <nano/store/account.hpp>
+#include <nano/store/reverse_iterator_templ.hpp>
+#include <nano/store/typed_iterator_templ.hpp>
+
+template class nano::store::typed_iterator<nano::account, nano::account_info>;
+template class nano::store::reverse_iterator<nano::store::typed_iterator<nano::account, nano::account_info>>;
 
 std::optional<nano::account_info> nano::store::account::get (store::transaction const & transaction, nano::account const & account)
 {
@@ -13,3 +18,15 @@ std::optional<nano::account_info> nano::store::account::get (store::transaction
 		return std::nullopt;
 	}
 }
+
+auto nano::store::account::rbegin (store::transaction const & tx) const -> reverse_iterator
+{
+	auto iter = end (tx);
+	--iter;
+	return reverse_iterator{ std::move (iter) };
+}
+
+auto nano::store::account::rend (transaction const & tx) const -> reverse_iterator
+{
+	return reverse_iterator{ end (tx) };
+}

nano/store/account.hpp

@@ -3,7 +3,8 @@
 #include <nano/lib/numbers.hpp>
 #include <nano/store/component.hpp>
 #include <nano/store/db_val_impl.hpp>
-#include <nano/store/iterator.hpp>
+#include <nano/store/reverse_iterator.hpp>
+#include <nano/store/typed_iterator.hpp>
 
 #include <functional>
 
@@ -20,19 +21,21 @@ namespace nano::store
 class account
 {
 public:
-	using iterator = store::iterator<nano::account, nano::account_info>;
+	using iterator = typed_iterator<nano::account, nano::account_info>;
+	using reverse_iterator = store::reverse_iterator<iterator>;
 
 public:
-	virtual void put (store::write_transaction const &, nano::account const &, nano::account_info const &) = 0;
-	virtual bool get (store::transaction const &, nano::account const &, nano::account_info &) = 0;
-	std::optional<nano::account_info> get (store::transaction const &, nano::account const &);
-	virtual void del (store::write_transaction const &, nano::account const &) = 0;
-	virtual bool exists (store::transaction const &, nano::account const &) = 0;
-	virtual size_t count (store::transaction const &) = 0;
-	virtual iterator begin (store::transaction const &, nano::account const &) const = 0;
-	virtual iterator begin (store::transaction const &) const = 0;
-	virtual iterator rbegin (store::transaction const &) const = 0;
-	virtual iterator end (store::transaction const & transaction_a) const = 0;
-	virtual void for_each_par (std::function<void (store::read_transaction const &, iterator, iterator)> const &) const = 0;
+	virtual void put (write_transaction const & tx, nano::account const &, nano::account_info const &) = 0;
+	virtual bool get (transaction const & tx, nano::account const &, nano::account_info &) = 0;
+	std::optional<nano::account_info> get (transaction const & tx, nano::account const &);
+	virtual void del (write_transaction const & tx, nano::account const &) = 0;
+	virtual bool exists (transaction const & tx, nano::account const &) = 0;
+	virtual size_t count (transaction const & tx) = 0;
+	virtual iterator begin (transaction const & tx, nano::account const &) const = 0;
+	virtual iterator begin (transaction const & tx) const = 0;
+	reverse_iterator rbegin (transaction const & tx) const;
+	reverse_iterator rend (transaction const & tx) const;
+	virtual iterator end (transaction const & tx) const = 0;
+	virtual void for_each_par (std::function<void (read_transaction const & tx, iterator, iterator)> const &) const = 0;
 };
 } // namespace nano::store

nano/store/block.cpp

@@ -1 +1,4 @@
 #include <nano/store/block.hpp>
+#include <nano/store/typed_iterator_templ.hpp>
+
+template class nano::store::typed_iterator<nano::block_hash, nano::store::block_w_sideband>;

nano/store/block.hpp

@@ -4,7 +4,7 @@
 #include <nano/lib/numbers.hpp>
 #include <nano/store/block_w_sideband.hpp>
 #include <nano/store/component.hpp>
-#include <nano/store/iterator.hpp>
+#include <nano/store/typed_iterator.hpp>
 
 #include <functional>
 #include <optional>
@@ -22,21 +22,21 @@ namespace nano::store
 class block
 {
 public:
-	using iterator = store::iterator<nano::block_hash, block_w_sideband>;
+	using iterator = typed_iterator<nano::block_hash, block_w_sideband>;
 
 public:
-	virtual void put (store::write_transaction const &, nano::block_hash const &, nano::block const &) = 0;
-	virtual void raw_put (store::write_transaction const &, std::vector<uint8_t> const &, nano::block_hash const &) = 0;
-	virtual std::optional<nano::block_hash> successor (store::transaction const &, nano::block_hash const &) const = 0;
-	virtual void successor_clear (store::write_transaction const &, nano::block_hash const &) = 0;
-	virtual std::shared_ptr<nano::block> get (store::transaction const &, nano::block_hash const &) const = 0;
-	virtual std::shared_ptr<nano::block> random (store::transaction const &) = 0;
-	virtual void del (store::write_transaction const &, nano::block_hash const &) = 0;
-	virtual bool exists (store::transaction const &, nano::block_hash const &) = 0;
-	virtual uint64_t count (store::transaction const &) = 0;
-	virtual iterator begin (store::transaction const &, nano::block_hash const &) const = 0;
-	virtual iterator begin (store::transaction const &) const = 0;
-	virtual iterator end (store::transaction const &) const = 0;
-	virtual void for_each_par (std::function<void (store::read_transaction const &, iterator, iterator)> const & action_a) const = 0;
+	virtual void put (write_transaction const & tx, nano::block_hash const &, nano::block const &) = 0;
+	virtual void raw_put (write_transaction const & tx, std::vector<uint8_t> const &, nano::block_hash const &) = 0;
+	virtual std::optional<nano::block_hash> successor (transaction const & tx, nano::block_hash const &) const = 0;
+	virtual void successor_clear (write_transaction const & tx, nano::block_hash const &) = 0;
+	virtual std::shared_ptr<nano::block> get (transaction const & tx, nano::block_hash const &) const = 0;
+	virtual std::shared_ptr<nano::block> random (transaction const & tx) = 0;
+	virtual void del (write_transaction const & tx, nano::block_hash const &) = 0;
+	virtual bool exists (transaction const & tx, nano::block_hash const &) = 0;
+	virtual uint64_t count (transaction const & tx) = 0;
+	virtual iterator begin (transaction const & tx, nano::block_hash const &) const = 0;
+	virtual iterator begin (transaction const & tx) const = 0;
+	virtual iterator end (transaction const & tx) const = 0;
+	virtual void for_each_par (std::function<void (read_transaction const & tx, iterator, iterator)> const & action_a) const = 0;
 };
 } // namespace nano::store

nano/store/confirmation_height.cpp

@@ -1,4 +1,7 @@
 #include <nano/store/confirmation_height.hpp>
+#include <nano/store/typed_iterator_templ.hpp>
+
+template class nano::store::typed_iterator<nano::account, nano::confirmation_height_info>;
 
 std::optional<nano::confirmation_height_info> nano::store::confirmation_height::get (store::transaction const & transaction, nano::account const & account)
 {

nano/store/confirmation_height.hpp

@@ -2,7 +2,7 @@
 
 #include <nano/lib/numbers.hpp>
 #include <nano/store/component.hpp>
-#include <nano/store/iterator.hpp>
+#include <nano/store/typed_iterator.hpp>
 
 #include <functional>
 
@@ -18,7 +18,7 @@ namespace nano::store
 class confirmation_height
 {
 public:
-	using iterator = store::iterator<nano::account, nano::confirmation_height_info>;
+	using iterator = typed_iterator<nano::account, nano::confirmation_height_info>;
 
 public:
 	virtual void put (store::write_transaction const & transaction_a, nano::account const & account_a, nano::confirmation_height_info const & confirmation_height_info_a) = 0;

nano/store/final_vote.cpp

@@ -1 +1,4 @@
 #include <nano/store/final_vote.hpp>
+#include <nano/store/typed_iterator_templ.hpp>
+
+template class nano::store::typed_iterator<nano::qualified_root, nano::block_hash>;