From a9bdc6e4c4e9ec73226e5b4d2261f54bb0b14cf1 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Mon, 4 Nov 2019 01:06:01 +0000 Subject: [PATCH] Improved performance for epee serialization: - Removed copy of field names in binary deserialization - Removed copy of array values in binary deserialization - Removed copy of string values in json deserialization - Removed unhelpful allocation in json string value parsing - Removed copy of blob data on binary and json serialization --- .../keyvalue_serialization_overloads.h | 24 +++------ .../include/storages/parserse_base_utils.h | 1 - .../epee/include/storages/portable_storage.h | 52 +++++++++++-------- .../include/storages/portable_storage_base.h | 8 +-- .../storages/portable_storage_from_bin.h | 6 +-- .../storages/portable_storage_from_json.h | 18 +++---- tests/unit_tests/net.cpp | 6 +-- 7 files changed, 56 insertions(+), 59 deletions(-) diff --git a/contrib/epee/include/serialization/keyvalue_serialization_overloads.h b/contrib/epee/include/serialization/keyvalue_serialization_overloads.h index fc8b90a2c..1f9d6b6d7 100644 --- a/contrib/epee/include/serialization/keyvalue_serialization_overloads.h +++ b/contrib/epee/include/serialization/keyvalue_serialization_overloads.h @@ -46,24 +46,12 @@ namespace epee namespace serialization { - //------------------------------------------------------------------------------------------------------------------- - template - static bool serialize_t_val(const t_type& d, t_storage& stg, typename t_storage::hsection hparent_section, const char* pname) - { - return stg.set_value(pname, d, hparent_section); - } - //------------------------------------------------------------------------------------------------------------------- - template - static bool unserialize_t_val(t_type& d, t_storage& stg, typename t_storage::hsection hparent_section, const char* pname) - { - return stg.get_value(pname, d, hparent_section); - } //------------------------------------------------------------------------------------------------------------------- template static bool serialize_t_val_as_blob(const t_type& d, t_storage& stg, typename t_storage::hsection hparent_section, const char* pname) { std::string blob((const char *)&d, sizeof(d)); - return stg.set_value(pname, blob, hparent_section); + return stg.set_value(pname, std::move(blob), hparent_section); } //------------------------------------------------------------------------------------------------------------------- template @@ -114,13 +102,15 @@ namespace epee template static bool serialize_stl_container_t_val (const stl_container& container, t_storage& stg, typename t_storage::hsection hparent_section, const char* pname) { + using value_type = typename stl_container::value_type; + if(!container.size()) return true; typename stl_container::const_iterator it = container.begin(); - typename t_storage::harray hval_array = stg.insert_first_value(pname, *it, hparent_section); + typename t_storage::harray hval_array = stg.insert_first_value(pname, value_type(*it), hparent_section); CHECK_AND_ASSERT_MES(hval_array, false, "failed to insert first value to storage"); it++; for(;it!= container.end();it++) - stg.insert_next_value(hval_array, *it); + stg.insert_next_value(hval_array, value_type(*it)); return true; } @@ -149,7 +139,7 @@ namespace epee *p_elem = v; p_elem++; } - return stg.set_value(pname, mb, hparent_section); + return stg.set_value(pname, std::move(mb), hparent_section); } //-------------------------------------------------------------------------------------------------------------------- template @@ -221,7 +211,7 @@ namespace epee template static bool kv_serialize(const t_type& d, t_storage& stg, typename t_storage::hsection hparent_section, const char* pname) { - return stg.set_value(pname, d, hparent_section); + return stg.set_value(pname, t_type(d), hparent_section); } //------------------------------------------------------------------------------------------------------------------- template diff --git a/contrib/epee/include/storages/parserse_base_utils.h b/contrib/epee/include/storages/parserse_base_utils.h index fe53628a5..8a498130c 100644 --- a/contrib/epee/include/storages/parserse_base_utils.h +++ b/contrib/epee/include/storages/parserse_base_utils.h @@ -157,7 +157,6 @@ namespace misc_utils while (fi != buf_end && ((lut[(uint8_t)*fi] & 32)) == 0) ++fi; val.assign(it, fi); - val.reserve(std::distance(star_end_string, buf_end)); it = fi; for(;it != buf_end;it++) { diff --git a/contrib/epee/include/storages/portable_storage.h b/contrib/epee/include/storages/portable_storage.h index d0e40d606..4b759a24f 100644 --- a/contrib/epee/include/storages/portable_storage.h +++ b/contrib/epee/include/storages/portable_storage.h @@ -28,6 +28,8 @@ #pragma once +#include + #include "misc_language.h" #include "portable_storage_base.h" #include "portable_storage_to_bin.h" @@ -59,7 +61,7 @@ namespace epee bool get_value(const std::string& value_name, t_value& val, hsection hparent_section); bool get_value(const std::string& value_name, storage_entry& val, hsection hparent_section); template - bool set_value(const std::string& value_name, const t_value& target, hsection hparent_section); + bool set_value(const std::string& value_name, t_value&& target, hsection hparent_section); //serial access for arrays of values -------------------------------------- //values @@ -68,9 +70,9 @@ namespace epee template bool get_next_value(harray hval_array, t_value& target); template - harray insert_first_value(const std::string& value_name, const t_value& target, hsection hparent_section); + harray insert_first_value(const std::string& value_name, t_value&& target, hsection hparent_section); template - bool insert_next_value(harray hval_array, const t_value& target); + bool insert_next_value(harray hval_array, t_value&& target); //sections harray get_first_section(const std::string& pSectionName, hsection& h_child_section, hsection hparent_section); bool get_next_section(harray hSecArray, hsection& h_child_section); @@ -94,7 +96,7 @@ namespace epee hsection get_root_section() {return &m_root;} storage_entry* find_storage_entry(const std::string& pentry_name, hsection psection); template - storage_entry* insert_new_entry_get_storage_entry(const std::string& pentry_name, hsection psection, const entry_type& entry); + storage_entry* insert_new_entry_get_storage_entry(const std::string& pentry_name, hsection psection, entry_type&& entry); hsection insert_new_section(const std::string& pentry_name, hsection psection); @@ -241,21 +243,22 @@ namespace epee } //--------------------------------------------------------------------------------------------------------------- template - bool portable_storage::set_value(const std::string& value_name, const t_value& v, hsection hparent_section) + bool portable_storage::set_value(const std::string& value_name, t_value&& v, hsection hparent_section) { - BOOST_MPL_ASSERT(( boost::mpl::contains::type, t_value> )); + using t_real_value = typename std::decay::type; + BOOST_MPL_ASSERT(( boost::mpl::contains::type, t_real_value> )); TRY_ENTRY(); if(!hparent_section) hparent_section = &m_root; storage_entry* pentry = find_storage_entry(value_name, hparent_section); if(!pentry) { - pentry = insert_new_entry_get_storage_entry(value_name, hparent_section, v); + pentry = insert_new_entry_get_storage_entry(value_name, hparent_section, std::forward(v)); if(!pentry) return false; return true; } - *pentry = storage_entry(v); + *pentry = std::forward(v); return true; CATCH_ENTRY("portable_storage::template<>set_value", false); } @@ -274,11 +277,12 @@ namespace epee } //--------------------------------------------------------------------------------------------------------------- template - storage_entry* portable_storage::insert_new_entry_get_storage_entry(const std::string& pentry_name, hsection psection, const entry_type& entry) + storage_entry* portable_storage::insert_new_entry_get_storage_entry(const std::string& pentry_name, hsection psection, entry_type&& entry) { + static_assert(std::is_rvalue_reference(), "unexpected copy of value"); TRY_ENTRY(); CHECK_AND_ASSERT(psection, nullptr); - auto ins_res = psection->m_entries.insert(std::pair(pentry_name, entry)); + auto ins_res = psection->m_entries.emplace(pentry_name, std::forward(entry)); return &ins_res.first->second; CATCH_ENTRY("portable_storage::insert_new_entry_get_storage_entry", nullptr); } @@ -362,41 +366,45 @@ namespace epee } //--------------------------------------------------------------------------------------------------------------- template - harray portable_storage::insert_first_value(const std::string& value_name, const t_value& target, hsection hparent_section) + harray portable_storage::insert_first_value(const std::string& value_name, t_value&& target, hsection hparent_section) { + using t_real_value = typename std::decay::type; + static_assert(std::is_rvalue_reference(), "unexpected copy of value"); TRY_ENTRY(); if(!hparent_section) hparent_section = &m_root; storage_entry* pentry = find_storage_entry(value_name, hparent_section); if(!pentry) { - pentry = insert_new_entry_get_storage_entry(value_name, hparent_section, array_entry(array_entry_t())); + pentry = insert_new_entry_get_storage_entry(value_name, hparent_section, array_entry(array_entry_t())); if(!pentry) return nullptr; } if(pentry->type() != typeid(array_entry)) - *pentry = storage_entry(array_entry(array_entry_t())); + *pentry = storage_entry(array_entry(array_entry_t())); array_entry& arr = boost::get(*pentry); - if(arr.type() != typeid(array_entry_t)) - arr = array_entry(array_entry_t()); + if(arr.type() != typeid(array_entry_t)) + arr = array_entry(array_entry_t()); - array_entry_t& arr_typed = boost::get >(arr); - arr_typed.insert_first_val(target); + array_entry_t& arr_typed = boost::get >(arr); + arr_typed.insert_first_val(std::forward(target)); return &arr; CATCH_ENTRY("portable_storage::insert_first_value", nullptr); } //--------------------------------------------------------------------------------------------------------------- template - bool portable_storage::insert_next_value(harray hval_array, const t_value& target) + bool portable_storage::insert_next_value(harray hval_array, t_value&& target) { + using t_real_value = typename std::decay::type; + static_assert(std::is_rvalue_reference(), "unexpected copy of value"); TRY_ENTRY(); CHECK_AND_ASSERT(hval_array, false); - CHECK_AND_ASSERT_MES(hval_array->type() == typeid(array_entry_t), - false, "unexpected type in insert_next_value: " << typeid(array_entry_t).name()); + CHECK_AND_ASSERT_MES(hval_array->type() == typeid(array_entry_t), + false, "unexpected type in insert_next_value: " << typeid(array_entry_t).name()); - array_entry_t& arr_typed = boost::get >(*hval_array); - arr_typed.insert_next_value(target); + array_entry_t& arr_typed = boost::get >(*hval_array); + arr_typed.insert_next_value(std::forward(target)); return true; CATCH_ENTRY("portable_storage::insert_next_value", false); } diff --git a/contrib/epee/include/storages/portable_storage_base.h b/contrib/epee/include/storages/portable_storage_base.h index ca7c81ddc..40c7524fb 100644 --- a/contrib/epee/include/storages/portable_storage_base.h +++ b/contrib/epee/include/storages/portable_storage_base.h @@ -111,16 +111,16 @@ namespace epee return (t_entry_type*)&(*(m_it++));//fuckoff } - t_entry_type& insert_first_val(const t_entry_type& v) + t_entry_type& insert_first_val(t_entry_type&& v) { m_array.clear(); m_it = m_array.end(); - return insert_next_value(v); + return insert_next_value(std::move(v)); } - t_entry_type& insert_next_value(const t_entry_type& v) + t_entry_type& insert_next_value(t_entry_type&& v) { - m_array.push_back(v); + m_array.push_back(std::move(v)); return m_array.back(); } diff --git a/contrib/epee/include/storages/portable_storage_from_bin.h b/contrib/epee/include/storages/portable_storage_from_bin.h index c0b6cc7b1..b39dc7c92 100644 --- a/contrib/epee/include/storages/portable_storage_from_bin.h +++ b/contrib/epee/include/storages/portable_storage_from_bin.h @@ -143,7 +143,7 @@ namespace epee //TODO: add some optimization here later while(size--) sa.m_array.push_back(read()); - return storage_entry(array_entry(sa)); + return storage_entry(array_entry(std::move(sa))); } inline @@ -213,7 +213,7 @@ namespace epee { RECURSION_LIMITATION(); section s;//use extra variable due to vs bug, line "storage_entry se(section()); " can't be compiled in visual studio - storage_entry se(s); + storage_entry se(std::move(s)); section& section_entry = boost::get
(se); read(section_entry); return se; @@ -268,7 +268,7 @@ namespace epee //read section name string std::string sec_name; read_sec_name(sec_name); - sec.m_entries.insert(std::make_pair(sec_name, load_storage_entry())); + sec.m_entries.emplace(std::move(sec_name), load_storage_entry()); } } inline diff --git a/contrib/epee/include/storages/portable_storage_from_json.h b/contrib/epee/include/storages/portable_storage_from_json.h index 3e3052541..2b2dc7ff9 100644 --- a/contrib/epee/include/storages/portable_storage_from_json.h +++ b/contrib/epee/include/storages/portable_storage_from_json.h @@ -128,20 +128,20 @@ namespace epee errno = 0; int64_t nval = strtoll(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - stg.set_value(name, nval, current_section); + stg.set_value(name, int64_t(nval), current_section); }else { errno = 0; uint64_t nval = strtoull(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - stg.set_value(name, nval, current_section); + stg.set_value(name, uint64_t(nval), current_section); } }else { errno = 0; double nval = strtod(val.data(), NULL); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - stg.set_value(name, nval, current_section); + stg.set_value(name, double(nval), current_section); } state = match_state_wonder_after_value; }else if(isalpha(*it) ) @@ -219,13 +219,13 @@ namespace epee errno = 0; int64_t nval = strtoll(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - h_array = stg.insert_first_value(name, nval, current_section); + h_array = stg.insert_first_value(name, int64_t(nval), current_section); }else { errno = 0; uint64_t nval = strtoull(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - h_array = stg.insert_first_value(name, nval, current_section); + h_array = stg.insert_first_value(name, uint64_t(nval), current_section); } CHECK_AND_ASSERT_THROW_MES(h_array, " failed to insert values section entry"); }else @@ -233,7 +233,7 @@ namespace epee errno = 0; double nval = strtod(val.data(), NULL); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - h_array = stg.insert_first_value(name, nval, current_section); + h_array = stg.insert_first_value(name, double(nval), current_section); CHECK_AND_ASSERT_THROW_MES(h_array, " failed to insert values section entry"); } @@ -310,20 +310,20 @@ namespace epee errno = 0; int64_t nval = strtoll(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - insert_res = stg.insert_next_value(h_array, nval); + insert_res = stg.insert_next_value(h_array, int64_t(nval)); }else { errno = 0; uint64_t nval = strtoull(val.data(), NULL, 10); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - insert_res = stg.insert_next_value(h_array, nval); + insert_res = stg.insert_next_value(h_array, uint64_t(nval)); } }else { errno = 0; double nval = strtod(val.data(), NULL); if (errno) throw std::runtime_error("Invalid number: " + std::string(val)); - insert_res = stg.insert_next_value(h_array, nval); + insert_res = stg.insert_next_value(h_array, double(nval)); } CHECK_AND_ASSERT_THROW_MES(insert_res, "Failed to insert next value"); state = match_state_array_after_value; diff --git a/tests/unit_tests/net.cpp b/tests/unit_tests/net.cpp index 262541bd2..221dc631d 100644 --- a/tests/unit_tests/net.cpp +++ b/tests/unit_tests/net.cpp @@ -271,7 +271,7 @@ TEST(tor_address, epee_serializev_v2) EXPECT_EQ(std::strlen(v2_onion), host.size()); host.push_back('k'); - EXPECT_TRUE(stg.set_value("host", host, stg.open_section("tor", nullptr, false))); + EXPECT_TRUE(stg.set_value("host", std::move(host), stg.open_section("tor", nullptr, false))); EXPECT_TRUE(command.load(stg)); // poor error reporting from `KV_SERIALIZE` } @@ -322,7 +322,7 @@ TEST(tor_address, epee_serializev_v3) EXPECT_EQ(std::strlen(v3_onion), host.size()); host.push_back('k'); - EXPECT_TRUE(stg.set_value("host", host, stg.open_section("tor", nullptr, false))); + EXPECT_TRUE(stg.set_value("host", std::move(host), stg.open_section("tor", nullptr, false))); EXPECT_TRUE(command.load(stg)); // poor error reporting from `KV_SERIALIZE` } @@ -373,7 +373,7 @@ TEST(tor_address, epee_serialize_unknown) EXPECT_EQ(std::strlen(net::tor_address::unknown_str()), host.size()); host.push_back('k'); - EXPECT_TRUE(stg.set_value("host", host, stg.open_section("tor", nullptr, false))); + EXPECT_TRUE(stg.set_value("host", std::move(host), stg.open_section("tor", nullptr, false))); EXPECT_TRUE(command.load(stg)); // poor error reporting from `KV_SERIALIZE` }