Avoid copy in get_item()

diodechain · Sep 13, 2024 · 5d77ca2 · 5d77ca2
1 parent e2fe3f0
commit 5d77ca2
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 18 deletions.
diff --git a/c_src/main.cpp b/c_src/main.cpp
@@ -108,8 +108,8 @@ int test3(int size, std::string ref, int change_size) {
 
     for (uint256_t &key : changes) {
         auto bkey = bin_t(key.data(), key.data() + 32);
-        pair_t ret = tree.get_item(bkey);
-        if (ret.key != bkey) {
+        pair_t* ret = tree.get_item(std::move(bkey));
+        if (ret == nullptr || ret->key != bkey) {
             std::cout << "tree.get_item(" << key.hex() << ") != " << key.hex() << std::endl;
             return 1;
         }

diff --git a/c_src/merkletree.cpp b/c_src/merkletree.cpp
@@ -179,13 +179,13 @@ bool map_contains(pair_list_t &map, bin_t &key) {
     return false;
 }
 
-pair_t map_get(pair_list_t &map, bin_t &key) {
+pair_t* map_get(pair_list_t &map, bin_t &key) {
     for (pair_t *pair : map) {
         if (pair->key == key) {
-            return *pair;
+            return pair;
         }
     }
-    return pair_t();
+    return nullptr;
 }
 
 void map_put(pair_list_t &map, pair_t &new_pair) {
@@ -422,19 +422,19 @@ proof_t Tree::get_proofs(bin_t& key) {
     return do_get_proofs(*this, *root, pair);
 }
 
-pair_t Tree::get_item(bin_t &key) {
+pair_t* Tree::get_item(bin_t &&key) {
     pair_t pair(key);
     return get_item(pair);
 }
 
-pair_t Tree::get_item(pair_t &pair) {
+pair_t* Tree::get_item(pair_t &pair) {
     return map_get(get_bucket(*root, pair).leaf_bucket, pair.key);
 }
 
 void Tree::difference(Tree &other, Tree &into) {
     each([&other, &into](pair_t &pair) {
-        pair_t other_pair = other.get_item(pair);
-        if (other_pair.value != pair.value) {
+        pair_t* other_pair = other.get_item(pair);
+        if (other_pair == nullptr || other_pair->value != pair.value) {
             into.insert_item(pair);
         }
     });

diff --git a/c_src/merkletree.hpp b/c_src/merkletree.hpp
@@ -128,8 +128,9 @@ class pair_t {
 public:
     pair_t() = default;
     pair_t(bin_t &key, uint256_t &value, uint256_t &key_hash) : key(key), value(value), key_hash(key_hash) { }
-    pair_t(bin_t &key, uint256_t &value) : key(key), value(value), key_hash(hash(key)) { }
-    pair_t(bin_t &key) : key(key), value(), key_hash(hash(key)) { }
+    pair_t(bin_t &_key, uint256_t &value) : key(_key), value(value), key_hash(hash(key)) { }
+    pair_t(bin_t &_key) : key(_key), value(), key_hash(hash(key)) { }
+    pair_t(bin_t &&_key) : key(_key), value(), key_hash(hash(key)) { }
     pair_t(Tree& /*tree*/) : key(), value(), key_hash() { }
     pair_t(const pair_t &other) = default;
     pair_t& operator=(const pair_t &other) = default;
@@ -286,8 +287,8 @@ class Tree {
         uint256_t null_value = {};
         insert_item(key, null_value); 
     }
-    pair_t get_item(bin_t &key);
-    pair_t get_item(pair_t &key);
+    pair_t* get_item(bin_t &&key);
+    pair_t* get_item(pair_t &key);
     proof_t get_proofs(bin_t &key);
     uint256_t root_hash();
     uint256_t* root_hashes();

diff --git a/c_src/nif.cpp b/c_src/nif.cpp
@@ -167,14 +167,18 @@ merkletree_get_item(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
 
     bin_t key;
     key.insert(key.end(), key_binary.data, key_binary.data + key_binary.size);
-    pair_t pair = mt->shared_state->tree.get_item(key);
+    pair_t* pair = mt->shared_state->tree.get_item(std::move(key));
     enif_release_binary(&key_binary);
 
 
+    if (pair == nullptr) {
+        return make_atom(env, "nil");
+    }
+
     // Should return {key, value, hash}
     ERL_NIF_TERM key_term = argv[1];
-    ERL_NIF_TERM value_term = make_binary(env, pair.value.data(), 32);
-    ERL_NIF_TERM hash_term = make_binary(env, pair.key_hash.data(), 32);
+    ERL_NIF_TERM value_term = make_binary(env, pair->value.data(), 32);
+    ERL_NIF_TERM hash_term = make_binary(env, pair->key_hash.data(), 32);
     ERL_NIF_TERM tuple = enif_make_tuple3(env, key_term, value_term, hash_term);
     return tuple;
 }
@@ -276,8 +280,8 @@ merkletree_difference(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
         auto pair1 = mt1->shared_state->tree.get_item(pair);
         auto pair2 = mt2->shared_state->tree.get_item(pair);
 
-        ERL_NIF_TERM value1_term = make_binary(env, pair1.value.data(), 32);
-        ERL_NIF_TERM value2_term = make_binary(env, pair2.value.data(), 32);
+        ERL_NIF_TERM value1_term = pair1 == nullptr ? make_atom(env, "nil") : make_binary(env, pair1->value.data(), 32);
+        ERL_NIF_TERM value2_term = pair2 == nullptr ? make_atom(env, "nil") : make_binary(env, pair2->value.data(), 32);
         ERL_NIF_TERM tuple = enif_make_tuple2(env, value1_term, value2_term);
         tuple = enif_make_tuple2(env, key_term, tuple);
         list = enif_make_list_cell(env, tuple, list);

diff --git a/scripts/cmerkle_bench2.exs b/scripts/cmerkle_bench2.exs
@@ -0,0 +1,36 @@
+# size = 10_000
+# ref = Base16.decode("0x96c2b5d03aa8e52230e74d9a08359c38c7608e5d9aba18773f3c90baf3806ccb")
+
+Application.ensure_all_started(:weak_ref)
+
+size = 1000
+ref = Base16.decode("0xe3545ad7961427b2a49d0b0a7b4a61e040acaefc432b69bcc1066b7c8c460a6b")
+# ref = Base16.decode("0x3b99d56aa16278d50e85a8ea0939e62180c4f0305b773775f1844c3303a41897") 100_000
+
+test_data =
+  Enum.map(1..size, fn idx ->
+    hash = Diode.hash("!#{idx}!")
+    {hash, hash}
+  end)
+
+for i <- 1..1000000 do
+  time_ms = :timer.tc(fn ->
+    tree_size = 1000
+    for _ <- 1..tree_size do
+      tree = CMerkleTree.insert_items(CMerkleTree.new(), test_data) |> CMerkleTree.lock()
+      for {key, value} <- test_data do
+        ^value = CMerkleTree.get(tree, key)
+      end
+
+      if CMerkleTree.root_hash(tree) != ref do
+        raise("Error #{Base16.encode(CMerkleTree.root_hash(tree))} != #{Base16.encode(ref)}")
+      end
+
+    end
+  end)
+  |> elem(0)
+  |> div(1000)
+
+  IO.puts("Run #{i} #{time_ms}ms")
+  :erlang.garbage_collect()
+end