From 8a450881b0106c6fcf852eb19cbf0423d9fac826 Mon Sep 17 00:00:00 2001 From: SergiySW Date: Sat, 15 Sep 2018 02:54:31 +0300 Subject: [PATCH 1/6] Remove unchecked_cache As it's used only n block_precessor, there is no reason to cache it for different write transacton --- rai/node/lmdb.cpp | 34 +++------------------------------- rai/node/lmdb.hpp | 1 - 2 files changed, 3 insertions(+), 32 deletions(-) diff --git a/rai/node/lmdb.cpp b/rai/node/lmdb.cpp index 54684fdde8..3d1da30f6c 100644 --- a/rai/node/lmdb.cpp +++ b/rai/node/lmdb.cpp @@ -1763,8 +1763,9 @@ void rai::mdb_store::unchecked_put (rai::transaction const & transaction_a, rai: // Inserting block if it wasn't found in database if (!exists) { - std::lock_guard lock (cache_mutex); - unchecked_cache.insert (std::make_pair (hash_a, block_a)); + mdb_val block (block_a); + auto status (mdb_put (env.tx (transaction_a), unchecked, rai::mdb_val (hash_a), block, 0)); + assert (status == 0); } } @@ -1786,13 +1787,6 @@ std::shared_ptr rai::mdb_store::vote_get (rai::transaction const & tr std::vector> rai::mdb_store::unchecked_get (rai::transaction const & transaction_a, rai::block_hash const & hash_a) { std::vector> result; - { - std::lock_guard lock (cache_mutex); - for (auto i (unchecked_cache.find (hash_a)), n (unchecked_cache.end ()); i != n && i->first == hash_a; ++i) - { - result.push_back (i->second); - } - } for (auto i (unchecked_begin (transaction_a, hash_a)), n (unchecked_end ()); i != n && rai::block_hash (i->first) == hash_a; i.next_dup ()) { std::shared_ptr block (i->second); @@ -1803,20 +1797,6 @@ std::vector> rai::mdb_store::unchecked_get (rai::tra void rai::mdb_store::unchecked_del (rai::transaction const & transaction_a, rai::block_hash const & hash_a, std::shared_ptr block_a) { - { - std::lock_guard lock (cache_mutex); - for (auto i (unchecked_cache.find (hash_a)), n (unchecked_cache.end ()); i != n && i->first == hash_a;) - { - if (*i->second == *block_a) - { - i = unchecked_cache.erase (i); - } - else - { - ++i; - } - } - } rai::mdb_val block (block_a); auto status (mdb_del (env.tx (transaction_a), unchecked, rai::mdb_val (hash_a), block)); assert (status == 0 || status == MDB_NOTFOUND); @@ -1868,17 +1848,9 @@ void rai::mdb_store::checksum_del (rai::transaction const & transaction_a, uint6 void rai::mdb_store::flush (rai::transaction const & transaction_a) { std::unordered_map> sequence_cache_l; - std::unordered_multimap> unchecked_cache_l; { std::lock_guard lock (cache_mutex); sequence_cache_l.swap (vote_cache); - unchecked_cache_l.swap (unchecked_cache); - } - for (auto & i : unchecked_cache_l) - { - mdb_val block (i.second); - auto status (mdb_put (env.tx (transaction_a), unchecked, rai::mdb_val (i.first), block, 0)); - assert (status == 0); } for (auto i (sequence_cache_l.begin ()), n (sequence_cache_l.end ()); i != n; ++i) { diff --git a/rai/node/lmdb.hpp b/rai/node/lmdb.hpp index 4918bdf4cc..c3161c8680 100644 --- a/rai/node/lmdb.hpp +++ b/rai/node/lmdb.hpp @@ -282,7 +282,6 @@ class mdb_store : public block_store rai::store_iterator> unchecked_begin (rai::transaction const &, rai::block_hash const &) override; rai::store_iterator> unchecked_end () override; size_t unchecked_count (rai::transaction const &) override; - std::unordered_multimap> unchecked_cache; void checksum_put (rai::transaction const &, uint64_t, uint8_t, rai::checksum const &) override; bool checksum_get (rai::transaction const &, uint64_t, uint8_t, rai::checksum &) override; From dbb8cf7391cd846d03c7efaf16bfbc55964abaa5 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Sat, 15 Sep 2018 12:17:20 -0700 Subject: [PATCH 2/6] Removing leaky abstraction in next_dup which also wasn't correctly updating the iterated value. --- rai/node/lmdb.cpp | 24 +++++------------------- rai/node/lmdb.hpp | 2 -- rai/secure/blockstore.hpp | 5 ----- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/rai/node/lmdb.cpp b/rai/node/lmdb.cpp index 3d1da30f6c..dfdd8fe038 100644 --- a/rai/node/lmdb.cpp +++ b/rai/node/lmdb.cpp @@ -428,7 +428,10 @@ template rai::store_iterator_impl & rai::mdb_iterator::operator++ () { assert (cursor != nullptr); - auto status (mdb_cursor_get (cursor, ¤t.first.value, ¤t.second.value, MDB_NEXT)); + unsigned flags; + auto flags_status (mdb_dbi_flags (mdb_cursor_txn (cursor), mdb_cursor_dbi (cursor), &flags)); + assert (flags_status == MDB_SUCCESS); + auto status (mdb_cursor_get (cursor, ¤t.first.value, ¤t.second.value, (flags & MDB_DUPSORT) != 0 ? MDB_NEXT_DUP : MDB_NEXT)); if (status == MDB_NOTFOUND) { clear (); @@ -471,17 +474,6 @@ bool rai::mdb_iterator::operator== (rai::store_iterator_impl const & return result; } -template -void rai::mdb_iterator::next_dup () -{ - assert (cursor != nullptr); - auto status (mdb_cursor_get (cursor, ¤t.first.value, ¤t.second.value, MDB_NEXT_DUP)); - if (status == MDB_NOTFOUND) - { - clear (); - } -} - template void rai::mdb_iterator::clear () { @@ -570,12 +562,6 @@ rai::store_iterator_impl & rai::mdb_merge_iterator::operator++ () return *this; } -template -void rai::mdb_merge_iterator::next_dup () -{ - least_iterator ().next_dup (); -} - template bool rai::mdb_merge_iterator::is_end_sentinal () const { @@ -1787,7 +1773,7 @@ std::shared_ptr rai::mdb_store::vote_get (rai::transaction const & tr std::vector> rai::mdb_store::unchecked_get (rai::transaction const & transaction_a, rai::block_hash const & hash_a) { std::vector> result; - for (auto i (unchecked_begin (transaction_a, hash_a)), n (unchecked_end ()); i != n && rai::block_hash (i->first) == hash_a; i.next_dup ()) + for (auto i (unchecked_begin (transaction_a, hash_a)), n (unchecked_end ()); i != n && rai::block_hash (i->first) == hash_a; ++i) { std::shared_ptr block (i->second); result.push_back (block); diff --git a/rai/node/lmdb.hpp b/rai/node/lmdb.hpp index c3161c8680..6a22c7858c 100644 --- a/rai/node/lmdb.hpp +++ b/rai/node/lmdb.hpp @@ -158,7 +158,6 @@ class mdb_iterator : public store_iterator_impl rai::store_iterator_impl & operator++ () override; std::pair * operator-> (); bool operator== (rai::store_iterator_impl const & other_a) const override; - void next_dup () override; bool is_end_sentinal () const override; void fill (std::pair &) const override; void clear (); @@ -187,7 +186,6 @@ class mdb_merge_iterator : public store_iterator_impl rai::store_iterator_impl & operator++ () override; std::pair * operator-> (); bool operator== (rai::store_iterator_impl const &) const override; - void next_dup () override; bool is_end_sentinal () const override; void fill (std::pair &) const override; void clear (); diff --git a/rai/secure/blockstore.hpp b/rai/secure/blockstore.hpp index 9dff05c55f..3c4393b84b 100644 --- a/rai/secure/blockstore.hpp +++ b/rai/secure/blockstore.hpp @@ -11,7 +11,6 @@ class store_iterator_impl virtual ~store_iterator_impl () = default; virtual rai::store_iterator_impl & operator++ () = 0; virtual bool operator== (rai::store_iterator_impl const & other_a) const = 0; - virtual void next_dup () = 0; virtual bool is_end_sentinal () const = 0; virtual void fill (std::pair &) const = 0; rai::store_iterator_impl & operator= (rai::store_iterator_impl const &) = delete; @@ -69,10 +68,6 @@ class store_iterator { return !(*this == other_a); } - void next_dup () - { - impl->next_dup (); - } private: std::pair current; From 1993cf64dca2a1c333ee602add4295301d52935a Mon Sep 17 00:00:00 2001 From: SergiySW Date: Sat, 15 Sep 2018 23:39:20 +0300 Subject: [PATCH 3/6] Test unchecked.multiple_get --- rai/core_test/block_store.cpp | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/rai/core_test/block_store.cpp b/rai/core_test/block_store.cpp index 3f9389a53c..4c1d502a6c 100644 --- a/rai/core_test/block_store.cpp +++ b/rai/core_test/block_store.cpp @@ -276,6 +276,58 @@ TEST (unchecked, double_put) ASSERT_EQ (block3.size (), 1); } +TEST (unchecked, multiple_get) +{ + bool init (false); + rai::mdb_store store (init, rai::unique_path ()); + ASSERT_TRUE (!init); + auto block1 (std::make_shared (4, 1, 2, rai::keypair ().prv, 4, 5)); + auto block2 (std::make_shared (3, 1, 2, rai::keypair ().prv, 4, 5)); + auto block3 (std::make_shared (5, 1, 2, rai::keypair ().prv, 4, 5)); + { + + auto transaction (store.tx_begin (true)); + store.unchecked_put (transaction, block1->previous (), block1); // unchecked1 + store.unchecked_put (transaction, block1->hash (), block1); // unchecked2 + store.unchecked_put (transaction, block2->previous (), block2); // unchecked3 + store.unchecked_put (transaction, block1->previous (), block2); // unchecked1 + store.unchecked_put (transaction, block1->hash (), block2); // unchecked2 + store.unchecked_put (transaction, block3->previous (), block3); + store.unchecked_put (transaction, block3->hash (), block3); // unchecked4 + store.unchecked_put (transaction, block1->previous (), block3); // unchecked1 + } + auto transaction (store.tx_begin ()); + auto unchecked_count (store.unchecked_count (transaction)); + ASSERT_EQ (unchecked_count, 8); + std::vector unchecked1; + auto unchecked1_blocks (store.unchecked_get (transaction, block1->previous ())); + ASSERT_EQ (unchecked1_blocks.size (), 3); + for (auto & i : unchecked1_blocks) + { + unchecked1.push_back (i->hash ()); + } + ASSERT_TRUE (std::find (unchecked1.begin (), unchecked1.end (), block1->hash ()) != unchecked1.end ()); + ASSERT_TRUE (std::find (unchecked1.begin (), unchecked1.end (), block2->hash ()) != unchecked1.end ()); + ASSERT_TRUE (std::find (unchecked1.begin (), unchecked1.end (), block3->hash ()) != unchecked1.end ()); + std::vector unchecked2; + auto unchecked2_blocks (store.unchecked_get (transaction, block1->hash ())); + ASSERT_EQ (unchecked2_blocks.size (), 2); + for (auto & i : unchecked2_blocks) + { + unchecked2.push_back (i->hash ()); + } + ASSERT_TRUE (std::find (unchecked2.begin (), unchecked2.end (), block1->hash ()) != unchecked2.end ()); + ASSERT_TRUE (std::find (unchecked2.begin (), unchecked2.end (), block2->hash ()) != unchecked2.end ()); + auto unchecked3 (store.unchecked_get (transaction, block2->previous ())); + ASSERT_EQ (unchecked3.size (), 1); + ASSERT_EQ (unchecked3[0]->hash (), block2->hash ()); + auto unchecked4 (store.unchecked_get (transaction, block3->hash ())); + ASSERT_EQ (unchecked4.size (), 1); + ASSERT_EQ (unchecked4[0]->hash (), block3->hash ()); + auto unchecked5 (store.unchecked_get (transaction, block2->hash ())); + ASSERT_EQ (unchecked5.size (), 0); +} + TEST (checksum, simple) { bool init (false); From 6188a9e03bc431c499a77c0688658931fb18d85c Mon Sep 17 00:00:00 2001 From: SergiySW Date: Sat, 15 Sep 2018 23:42:52 +0300 Subject: [PATCH 4/6] Simplify ++ iterator --- rai/node/lmdb.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rai/node/lmdb.cpp b/rai/node/lmdb.cpp index dfdd8fe038..82c6195cf8 100644 --- a/rai/node/lmdb.cpp +++ b/rai/node/lmdb.cpp @@ -428,10 +428,7 @@ template rai::store_iterator_impl & rai::mdb_iterator::operator++ () { assert (cursor != nullptr); - unsigned flags; - auto flags_status (mdb_dbi_flags (mdb_cursor_txn (cursor), mdb_cursor_dbi (cursor), &flags)); - assert (flags_status == MDB_SUCCESS); - auto status (mdb_cursor_get (cursor, ¤t.first.value, ¤t.second.value, (flags & MDB_DUPSORT) != 0 ? MDB_NEXT_DUP : MDB_NEXT)); + auto status (mdb_cursor_get (cursor, ¤t.first.value, ¤t.second.value, MDB_NEXT)); if (status == MDB_NOTFOUND) { clear (); From 06e7ae0fe27348e9b03ff422ad94da83168836cc Mon Sep 17 00:00:00 2001 From: SergiySW Date: Sat, 15 Sep 2018 23:44:20 +0300 Subject: [PATCH 5/6] Simplify unchecked_get --- rai/node/lmdb.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rai/node/lmdb.cpp b/rai/node/lmdb.cpp index 82c6195cf8..d48b95aca5 100644 --- a/rai/node/lmdb.cpp +++ b/rai/node/lmdb.cpp @@ -1770,8 +1770,9 @@ std::shared_ptr rai::mdb_store::vote_get (rai::transaction const & tr std::vector> rai::mdb_store::unchecked_get (rai::transaction const & transaction_a, rai::block_hash const & hash_a) { std::vector> result; - for (auto i (unchecked_begin (transaction_a, hash_a)), n (unchecked_end ()); i != n && rai::block_hash (i->first) == hash_a; ++i) + for (auto i (unchecked_begin (transaction_a, hash_a)), n (unchecked_begin (transaction_a, hash_a.number () + 1)); i != n; ++i) { + assert (rai::block_hash (i->first) == hash_a); std::shared_ptr block (i->second); result.push_back (block); } From 1913a60e6a0f3188dbe042ff3ee4947e3153a947 Mon Sep 17 00:00:00 2001 From: SergiySW Date: Sat, 15 Sep 2018 23:55:09 +0300 Subject: [PATCH 6/6] Typo --- rai/core_test/block_store.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/rai/core_test/block_store.cpp b/rai/core_test/block_store.cpp index 4c1d502a6c..a369cef4a8 100644 --- a/rai/core_test/block_store.cpp +++ b/rai/core_test/block_store.cpp @@ -285,7 +285,6 @@ TEST (unchecked, multiple_get) auto block2 (std::make_shared (3, 1, 2, rai::keypair ().prv, 4, 5)); auto block3 (std::make_shared (5, 1, 2, rai::keypair ().prv, 4, 5)); { - auto transaction (store.tx_begin (true)); store.unchecked_put (transaction, block1->previous (), block1); // unchecked1 store.unchecked_put (transaction, block1->hash (), block1); // unchecked2