From 6c379027c67acd182a35995511478c99ad59ac3b Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 25 Feb 2021 17:37:39 -0500 Subject: [PATCH 1/2] cache key values in iterator store in db_context_rocksdb --- .../backing_store/db_context_rocksdb.cpp | 64 +++++++++++-------- .../backing_store/db_key_value_iter_store.hpp | 14 ++++ 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/libraries/chain/backing_store/db_context_rocksdb.cpp b/libraries/chain/backing_store/db_context_rocksdb.cpp index 689f68e89b6..f10f6a0c7a5 100644 --- a/libraries/chain/backing_store/db_context_rocksdb.cpp +++ b/libraries/chain/backing_store/db_context_rocksdb.cpp @@ -200,16 +200,16 @@ namespace eosio { namespace chain { namespace backing_store { const unique_table t { receiver, scope_name, table_name }; const auto table_ei = primary_iter_store.cache_table(t); - return primary_iter_store.add(primary_key_iter(table_ei, id, payer)); + const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, id, payer)); + primary_iter_store.set_value(store_itr, pp.value, pp.value_size); + return store_itr; } void db_context_rocksdb::db_update_i64(int32_t itr, account_name payer, const char* value , size_t value_size) { const auto& key_store = primary_iter_store.get(itr); const auto& table_store = primary_iter_store.get_table(key_store); EOS_ASSERT( table_store.contract == receiver, table_access_violation, "db access violation" ); - const auto old_key_value = get_primary_key_value(receiver, table_store.scope, table_store.table, key_store.primary); - - EOS_ASSERT( old_key_value.value, db_rocksdb_invalid_operation_exception, + EOS_ASSERT( key_store.value, db_rocksdb_invalid_operation_exception, "invariant failure in db_update_i64, iter store found to update but nothing in database"); // copy locally, since below the key_store memory will be changed @@ -218,11 +218,11 @@ namespace eosio { namespace chain { namespace backing_store { payer = old_payer; } - const payer_payload old_pp{*old_key_value.value}; - const auto old_value_actual_size = old_pp.value_size; + const auto old_value_actual_size = key_store.value->size(); const payer_payload pp{payer, value, value_size}; - set_value(old_key_value.full_key, pp); + prefix_bundle old_key = get_primary_slice_in_primaries(table_store.contract, table_store.scope,table_store.table, key_store.primary); + set_value(old_key.full_key, pp); std::string event_id; auto dm_logger = context.control.get_deep_mind_logger(); @@ -250,17 +250,17 @@ namespace eosio { namespace chain { namespace backing_store { if (dm_logger != nullptr) { db_context::log_row_update(*dm_logger, context.get_action_id(), table_store.contract, table_store.scope, table_store.table, old_payer, payer, name(key_store.primary), - old_key_value.value->data(), old_key_value.value->size(), value, value_size); + key_store.value->data(), old_value_actual_size, value, value_size); } + + primary_iter_store.set_value(itr, value, value_size); } void db_context_rocksdb::db_remove_i64(int32_t itr) { const auto& key_store = primary_iter_store.get(itr); const auto& table_store = primary_iter_store.get_table(key_store); EOS_ASSERT( table_store.contract == receiver, table_access_violation, "db access violation" ); - const auto old_key_value = get_primary_key_value(receiver, table_store.scope, table_store.table, key_store.primary); - - EOS_ASSERT( old_key_value.value, db_rocksdb_invalid_operation_exception, + EOS_ASSERT( key_store.value, db_rocksdb_invalid_operation_exception, "invariant failure in db_remove_i64, iter store found to update but nothing in database"); const auto old_payer = key_store.payer; @@ -271,36 +271,31 @@ namespace eosio { namespace chain { namespace backing_store { event_id = db_context::table_event(table_store.contract, table_store.scope, table_store.table, name(key_store.primary)); } - payer_payload pp(*old_key_value.value); - update_db_usage( old_payer, -(pp.value_size + db_key_value_any_lookup::overhead), db_context::row_rem_trace(context.get_action_id(), std::move(event_id)) ); + update_db_usage( old_payer, -(key_store.value->size() + db_key_value_any_lookup::overhead), db_context::row_rem_trace(context.get_action_id(), std::move(event_id)) ); if (dm_logger != nullptr) { db_context::log_row_remove(*dm_logger, context.get_action_id(), table_store.contract, table_store.scope, - table_store.table, old_payer, name(key_store.primary), pp.value, - pp.value_size); + table_store.table, old_payer, name(key_store.primary), key_store.value->data(), + key_store.value->size()); } - current_session.erase(old_key_value.full_key); - primary_lookup.remove_table_if_empty(old_key_value.full_key); + prefix_bundle old_key = get_primary_slice_in_primaries(table_store.contract, table_store.scope,table_store.table, key_store.primary); + current_session.erase(old_key.full_key); + primary_lookup.remove_table_if_empty(old_key.full_key); primary_iter_store.remove(itr); // don't use key_store anymore } int32_t db_context_rocksdb::db_get_i64(int32_t itr, char* value , size_t value_size) { const auto& key_store = primary_iter_store.get(itr); - const auto& table_store = primary_iter_store.get_table(key_store); - const auto old_key_value = get_primary_key_value(table_store.contract, table_store.scope, table_store.table, key_store.primary); - - EOS_ASSERT( old_key_value.value, db_rocksdb_invalid_operation_exception, + EOS_ASSERT( key_store.value, db_rocksdb_invalid_operation_exception, "invariant failure in db_get_i64, iter store found to update but nothing in database"); - payer_payload pp {*old_key_value.value}; - const char* const actual_value = pp.value; - const size_t actual_size = pp.value_size; + const size_t actual_size = key_store.value->size(); if (value_size == 0) { return actual_size; } const size_t copy_size = std::min(value_size, actual_size); - memcpy( value, actual_value, copy_size ); + memcpy( value, key_store.value->data(), copy_size ); return copy_size; } @@ -394,6 +389,7 @@ namespace eosio { namespace chain { namespace backing_store { const account_name found_payer = pp.payer; const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, id, found_payer)); + primary_iter_store.set_value(store_itr, pp.value, pp.value_size); return store_itr; } @@ -708,8 +704,15 @@ namespace eosio { namespace chain { namespace backing_store { ("func", calling_func)); const account_name found_payer = payer_payload(*(*session_iter).second).payer; + const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, found_key, found_payer)); + + // value pointed by the iterator might have changed. need to cache from the source. + const backing_store::unique_table* table_store = primary_iter_store.find_table_by_end_iterator(table_ei); + const auto current_key_value = get_primary_key_value(table_store->contract, table_store->scope, table_store->table, found_key); + payer_payload pp {*current_key_value.value}; + primary_iter_store.set_value(store_itr, pp.value, pp.value_size); - return primary_iter_store.add(primary_key_iter(table_ei, found_key, found_payer)); + return store_itr; } // returns the exact iterator and the bounding key (type) @@ -772,7 +775,14 @@ namespace eosio { namespace chain { namespace backing_store { primary_key = key; } - return primary_iter_store.add(primary_key_iter(table_ei, *primary_key, found_payer)); + const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, *primary_key, found_payer)); + + // value pointed by the iterator might have changed. need to cache from the source. + const auto current_key_value = get_primary_key_value(code, scope, table, *primary_key); + payer_payload pp {*current_key_value.value}; + primary_iter_store.set_value(store_itr, pp.value, pp.value_size); + + return store_itr; } std::unique_ptr create_db_rocksdb_context(apply_context& context, name receiver, diff --git a/libraries/chain/include/eosio/chain/backing_store/db_key_value_iter_store.hpp b/libraries/chain/include/eosio/chain/backing_store/db_key_value_iter_store.hpp index d9f921325ff..c22a5e1d01c 100644 --- a/libraries/chain/include/eosio/chain/backing_store/db_key_value_iter_store.hpp +++ b/libraries/chain/include/eosio/chain/backing_store/db_key_value_iter_store.hpp @@ -17,6 +17,7 @@ struct secondary_key { T secondary; uint64_t primary; account_name payer; + std::optional value; }; inline bool operator<(const unique_table& lhs, const unique_table& rhs) { @@ -138,6 +139,19 @@ class db_key_value_iter_store { return _iterator_to_object.size() - 1; } + void set_value(int iterator, const char* data, std::size_t size) { + validate_object_iterator(iterator, "dereference of end iterator"); + // grab a const ref, to ensure that we are returning a reference to the actual object in the vector + auto& result = _iterator_to_object[iterator]; + EOS_ASSERT( result, table_operation_not_permitted, "dereference of deleted object" ); + if (!result->value) { + result->value = bytes(size); + } else { + result->value->resize(size); + } + std::memcpy(result->value->data(), data, size); + } + private: void validate_object_iterator(int iterator, const char* explanation_for_no_end_iterators) const { EOS_ASSERT( iterator != invalid_iterator(), invalid_table_iterator, "invalid iterator" ); From 3511519281f08bf574382426dd54c92c50c2e5c2 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 18 Mar 2021 09:26:57 -0400 Subject: [PATCH 2/2] Fix temporary shared_bytes and use value from iterator directly --- .../chain/backing_store/db_context_rocksdb.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/libraries/chain/backing_store/db_context_rocksdb.cpp b/libraries/chain/backing_store/db_context_rocksdb.cpp index f10f6a0c7a5..b2c82762570 100644 --- a/libraries/chain/backing_store/db_context_rocksdb.cpp +++ b/libraries/chain/backing_store/db_context_rocksdb.cpp @@ -703,13 +703,10 @@ namespace eosio { namespace chain { namespace backing_store { "invariant failure in ${func}, iter store found to update but no primary keys in database", ("func", calling_func)); - const account_name found_payer = payer_payload(*(*session_iter).second).payer; + auto value = *(*session_iter).second; // copy the value out so it remains persistent + payer_payload pp(value); + const account_name found_payer = pp.payer; const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, found_key, found_payer)); - - // value pointed by the iterator might have changed. need to cache from the source. - const backing_store::unique_table* table_store = primary_iter_store.find_table_by_end_iterator(table_ei); - const auto current_key_value = get_primary_key_value(table_store->contract, table_store->scope, table_store->table, found_key); - payer_payload pp {*current_key_value.value}; primary_iter_store.set_value(store_itr, pp.value, pp.value_size); return store_itr; @@ -766,7 +763,9 @@ namespace eosio { namespace chain { namespace backing_store { primary_key = id; } - const account_name found_payer = payer_payload(*(*session_iter).second).payer; + auto value = *(*session_iter).second; // copy the value out so it remains persistent + payer_payload pp(value); + const account_name found_payer = pp.payer; if (!primary_key) { uint64_t key = 0; const auto valid = db_key_value_format::get_primary_key((*session_iter).first, desired_type_prefix, key); @@ -774,12 +773,7 @@ namespace eosio { namespace chain { namespace backing_store { "invariant failure in find_i64, primary key already verified but method indicates it is not a primary key"); primary_key = key; } - const auto store_itr = primary_iter_store.add(primary_key_iter(table_ei, *primary_key, found_payer)); - - // value pointed by the iterator might have changed. need to cache from the source. - const auto current_key_value = get_primary_key_value(code, scope, table, *primary_key); - payer_payload pp {*current_key_value.value}; primary_iter_store.set_value(store_itr, pp.value, pp.value_size); return store_itr;