From b585a7f40818c9ea3bec3608f725f62f6e8c5268 Mon Sep 17 00:00:00 2001 From: j-berman Date: Thu, 25 Jul 2024 16:30:50 -0700 Subject: [PATCH] Remove copy in get_tree_extension and better named funcs --- src/blockchain_db/blockchain_db.h | 2 +- src/blockchain_db/lmdb/db_lmdb.cpp | 16 ++++++++-------- src/blockchain_db/lmdb/db_lmdb.h | 4 ++-- src/blockchain_db/testdb.h | 2 +- src/fcmp/curve_trees.cpp | 8 +++----- src/fcmp/curve_trees.h | 2 +- tests/unit_tests/curve_trees.cpp | 16 ++++++++++++---- 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 7ef14df82..309a36641 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -1780,7 +1780,7 @@ public: // TODO: description and make private virtual void grow_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, - const std::vector &new_leaves) = 0; + std::vector &&new_leaves) = 0; virtual void trim_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, const uint64_t trim_n_leaf_tuples) = 0; diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 9290998c6..36c89ab50 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -846,8 +846,8 @@ void BlockchainLMDB::add_block(const block& blk, size_t block_weight, uint64_t l } // Grow the tree with outputs that unlock at this block height - const auto unlocked_leaf_tuples = this->get_locked_leaf_tuples_at_block_id(m_height); - this->grow_tree(fcmp::curve_trees::curve_trees_v1, unlocked_leaf_tuples); + auto unlocked_leaf_tuples = this->get_leaf_tuples_at_unlock_block_id(m_height); + this->grow_tree(fcmp::curve_trees::curve_trees_v1, std::move(unlocked_leaf_tuples)); // Now that we've used the unlocked leaves to grow the tree, we can delete them from the locked leaves table this->del_locked_leaf_tuples_at_block_id(m_height); @@ -1364,7 +1364,7 @@ void BlockchainLMDB::remove_spent_key(const crypto::key_image& k_image) } void BlockchainLMDB::grow_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, - const std::vector &new_leaves) + std::vector &&new_leaves) { if (new_leaves.empty()) return; @@ -1384,7 +1384,7 @@ void BlockchainLMDB::grow_tree(const fcmp::curve_trees::CurveTreesV1 &curve_tree const auto last_hashes = this->get_tree_last_hashes(); // Use the number of leaf tuples and the existing last hashes to get a struct we can use to extend the tree - const auto tree_extension = curve_trees.get_tree_extension(old_n_leaf_tuples, last_hashes, new_leaves); + const auto tree_extension = curve_trees.get_tree_extension(old_n_leaf_tuples, last_hashes, std::move(new_leaves)); // Insert the leaves // TODO: grow_leaves @@ -2206,7 +2206,7 @@ bool BlockchainLMDB::audit_layer(const C_CHILD &c_child, chunk_width); } -std::vector BlockchainLMDB::get_locked_leaf_tuples_at_block_id( +std::vector BlockchainLMDB::get_leaf_tuples_at_unlock_block_id( uint64_t block_id) { LOG_PRINT_L3("BlockchainLMDB::" << __func__); @@ -6885,9 +6885,9 @@ void BlockchainLMDB::migrate_5_6() } } - // Get all the locked outputs at that height - const auto leaf_tuples = this->get_locked_leaf_tuples_at_block_id(i); - this->grow_tree(fcmp::curve_trees::curve_trees_v1, leaf_tuples); + // Get the leaf tuples that unlock at the given block + auto unlocked_leaf_tuples = this->get_leaf_tuples_at_unlock_block_id(i); + this->grow_tree(fcmp::curve_trees::curve_trees_v1, std::move(unlocked_leaf_tuples)); // Now that we've used the unlocked leaves to grow the tree, we can delete them from the locked leaves table this->del_locked_leaf_tuples_at_block_id(i); diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index b363077bc..d490ce6e6 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -369,7 +369,7 @@ public: // make private virtual void grow_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, - const std::vector &new_leaves); + std::vector &&new_leaves); virtual void trim_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, const uint64_t trim_n_leaf_tuples); @@ -450,7 +450,7 @@ private: const uint64_t child_chunk_idx, const uint64_t chunk_width) const; - std::vector get_locked_leaf_tuples_at_block_id(uint64_t block_id); + std::vector get_leaf_tuples_at_unlock_block_id(uint64_t block_id); void del_locked_leaf_tuples_at_block_id(uint64_t block_id); diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h index 48fab6076..bdbd6f2ad 100644 --- a/src/blockchain_db/testdb.h +++ b/src/blockchain_db/testdb.h @@ -117,7 +117,7 @@ public: virtual void add_spent_key(const crypto::key_image& k_image) override {} virtual void remove_spent_key(const crypto::key_image& k_image) override {} virtual void grow_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, - const std::vector &new_leaves) override {}; + std::vector &&new_leaves) override {}; virtual void trim_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, const uint64_t trim_n_leaf_tuples) override {}; virtual bool audit_tree(const fcmp::curve_trees::CurveTreesV1 &curve_trees, const uint64_t expected_n_leaf_tuples) const override { return false; }; diff --git a/src/fcmp/curve_trees.cpp b/src/fcmp/curve_trees.cpp index 0c48f55d2..bcdc85c7c 100644 --- a/src/fcmp/curve_trees.cpp +++ b/src/fcmp/curve_trees.cpp @@ -732,7 +732,7 @@ template typename CurveTrees::TreeExtension CurveTrees::get_tree_extension( const uint64_t old_n_leaf_tuples, const LastHashes &existing_last_hashes, - const std::vector &new_leaf_tuples) const + std::vector &&new_leaf_tuples) const { TreeExtension tree_extension; @@ -748,15 +748,13 @@ typename CurveTrees::TreeExtension CurveTrees::get_tree_extensio tree_extension.leaves.start_leaf_tuple_idx = grow_layer_instructions.old_total_children / LEAF_TUPLE_SIZE; // Sort the leaves by order they appear in the chain - // TODO: don't copy here - std::vector sorted_leaf_tuples = new_leaf_tuples; const auto sort_fn = [](const LeafTupleContext &a, const LeafTupleContext &b) { return a.output_id < b.output_id; }; - std::sort(sorted_leaf_tuples.begin(), sorted_leaf_tuples.end(), sort_fn); + std::sort(new_leaf_tuples.begin(), new_leaf_tuples.end(), sort_fn); // Copy the sorted leaves into the tree extension struct // TODO: don't copy here tree_extension.leaves.tuples.reserve(new_leaf_tuples.size()); - for (const auto &leaf : sorted_leaf_tuples) + for (const auto &leaf : new_leaf_tuples) { tree_extension.leaves.tuples.emplace_back(LeafTuple{ .O_x = leaf.leaf_tuple.O_x, diff --git a/src/fcmp/curve_trees.h b/src/fcmp/curve_trees.h index 3230d41ff..fcad2a0d9 100644 --- a/src/fcmp/curve_trees.h +++ b/src/fcmp/curve_trees.h @@ -238,7 +238,7 @@ public: // leaves to add to the tree, and return a tree extension struct that can be used to extend a tree TreeExtension get_tree_extension(const uint64_t old_n_leaf_tuples, const LastHashes &existing_last_hashes, - const std::vector &new_leaf_tuples) const; + std::vector &&new_leaf_tuples) const; // Get instructions useful for trimming all existing layers in the tree std::vector get_trim_instructions( diff --git a/tests/unit_tests/curve_trees.cpp b/tests/unit_tests/curve_trees.cpp index d9879ca43..5a3bb5555 100644 --- a/tests/unit_tests/curve_trees.cpp +++ b/tests/unit_tests/curve_trees.cpp @@ -775,11 +775,13 @@ static bool grow_tree(CurveTreesV1 &curve_trees, global_tree.log_last_hashes(last_hashes); + auto new_leaf_tuples = generate_random_leaves(curve_trees, old_n_leaf_tuples, new_n_leaf_tuples); + // Get a tree extension object to the existing tree using randomly generated leaves // - The tree extension includes all elements we'll need to add to the existing tree when adding the new leaves const auto tree_extension = curve_trees.get_tree_extension(old_n_leaf_tuples, last_hashes, - generate_random_leaves(curve_trees, old_n_leaf_tuples, new_n_leaf_tuples)); + std::move(new_leaf_tuples)); global_tree.log_tree_extension(tree_extension); @@ -857,14 +859,18 @@ static bool grow_tree_db(const std::size_t init_leaves, LOG_PRINT_L1("Adding " << init_leaves << " leaves to db, then extending by " << ext_leaves << " leaves"); - test_db.m_db->grow_tree(curve_trees, generate_random_leaves(curve_trees, 0, init_leaves)); + auto init_leaf_tuples = generate_random_leaves(curve_trees, 0, init_leaves); + + test_db.m_db->grow_tree(curve_trees, std::move(init_leaf_tuples)); CHECK_AND_ASSERT_MES(test_db.m_db->audit_tree(curve_trees, init_leaves), false, "failed to add initial leaves to db"); MDEBUG("Successfully added initial " << init_leaves << " leaves to db, extending by " << ext_leaves << " leaves"); - test_db.m_db->grow_tree(curve_trees, generate_random_leaves(curve_trees, init_leaves, ext_leaves)); + auto ext_leaf_tuples = generate_random_leaves(curve_trees, init_leaves, ext_leaves); + + test_db.m_db->grow_tree(curve_trees, std::move(ext_leaf_tuples)); CHECK_AND_ASSERT_MES(test_db.m_db->audit_tree(curve_trees, init_leaves + ext_leaves), false, "failed to extend tree in db"); @@ -886,7 +892,9 @@ static bool trim_tree_db(const std::size_t init_leaves, LOG_PRINT_L1("Adding " << init_leaves << " leaves to db, then trimming by " << trim_leaves << " leaves"); - test_db.m_db->grow_tree(curve_trees, generate_random_leaves(curve_trees, 0, init_leaves)); + auto init_leaf_tuples = generate_random_leaves(curve_trees, 0, init_leaves); + + test_db.m_db->grow_tree(curve_trees, std::move(init_leaf_tuples)); CHECK_AND_ASSERT_MES(test_db.m_db->audit_tree(curve_trees, init_leaves), false, "failed to add initial leaves to db");