From f641c08e804bfdc2804f6053a070b4e792cf1306 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 5 Oct 2022 16:35:16 -0300 Subject: [PATCH] llarp/profiling: refactor to use oxenc producer/consumer No more llarp_buffer_t here! (I was tracking down a segfault which led me in here and it was easier to rewrite this to use bt_dict_{consumer,producer} than to decipher all the cursed llarp_buffer_t and bencode callback nest). --- llarp/profiling.cpp | 213 ++++++++++++++++++++--------------------- llarp/profiling.hpp | 38 +++++--- llarp/util/aligned.hpp | 6 ++ 3 files changed, 136 insertions(+), 121 deletions(-) diff --git a/llarp/profiling.cpp b/llarp/profiling.cpp index ce57f42fd..45af9b9ff 100644 --- a/llarp/profiling.cpp +++ b/llarp/profiling.cpp @@ -1,53 +1,51 @@ #include "profiling.hpp" +#include +#include -#include -#include "util/fs.hpp" +#include "util/file.hpp" +#include "util/logging.hpp" + +using oxenc::bt_dict_consumer; +using oxenc::bt_dict_producer; namespace llarp { - bool - RouterProfile::BEncode(llarp_buffer_t* buf) const - { - if (!bencode_start_dict(buf)) - return false; + static auto logcat = log::Cat("profiling"); - if (!BEncodeWriteDictInt("g", connectGoodCount, buf)) - return false; - if (!BEncodeWriteDictInt("p", pathSuccessCount, buf)) - return false; - if (!BEncodeWriteDictInt("q", pathTimeoutCount, buf)) - return false; - if (!BEncodeWriteDictInt("s", pathFailCount, buf)) - return false; - if (!BEncodeWriteDictInt("t", connectTimeoutCount, buf)) - return false; - if (!BEncodeWriteDictInt("u", lastUpdated.count(), buf)) - return false; - if (!BEncodeWriteDictInt("v", version, buf)) - return false; + RouterProfile::RouterProfile(bt_dict_consumer dict) + { + BDecode(std::move(dict)); + } - return bencode_end(buf); + void + RouterProfile::BEncode(bt_dict_producer& dict) const + { + dict.append("g", connectGoodCount); + dict.append("p", pathSuccessCount); + dict.append("q", pathTimeoutCount); + dict.append("s", pathFailCount); + dict.append("t", connectTimeoutCount); + dict.append("u", lastUpdated.count()); + dict.append("v", version); } - bool - RouterProfile::DecodeKey(const llarp_buffer_t& k, llarp_buffer_t* buf) + void + RouterProfile::BDecode(bt_dict_consumer dict) { - bool read = false; - if (!BEncodeMaybeReadDictInt("g", connectGoodCount, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("t", connectTimeoutCount, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("u", lastUpdated, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("v", version, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("s", pathFailCount, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("p", pathSuccessCount, read, k, buf)) - return false; - if (!BEncodeMaybeReadDictInt("q", pathTimeoutCount, read, k, buf)) - return false; - return read; + if (dict.skip_until("g")) + connectGoodCount = dict.consume_integer(); + if (dict.skip_until("p")) + pathSuccessCount = dict.consume_integer(); + if (dict.skip_until("q")) + pathTimeoutCount = dict.consume_integer(); + if (dict.skip_until("s")) + pathFailCount = dict.consume_integer(); + if (dict.skip_until("t")) + connectTimeoutCount = dict.consume_integer(); + if (dict.skip_until("u")) + lastUpdated = llarp_time_t{dict.consume_integer()}; + if (dict.skip_until("v")) + version = dict.consume_integer(); } void @@ -156,23 +154,26 @@ namespace llarp Profiling::Tick() { util::Lock lock(m_ProfilesMutex); - std::for_each(m_Profiles.begin(), m_Profiles.end(), [](auto& item) { item.second.Tick(); }); + for (auto& [rid, profile] : m_Profiles) + profile.Tick(); } void Profiling::MarkConnectTimeout(const RouterID& r) { util::Lock lock{m_ProfilesMutex}; - m_Profiles[r].connectTimeoutCount += 1; - m_Profiles[r].lastUpdated = llarp::time_now_ms(); + auto& profile = m_Profiles[r]; + profile.connectTimeoutCount += 1; + profile.lastUpdated = llarp::time_now_ms(); } void Profiling::MarkConnectSuccess(const RouterID& r) { util::Lock lock{m_ProfilesMutex}; - m_Profiles[r].connectGoodCount += 1; - m_Profiles[r].lastUpdated = llarp::time_now_ms(); + auto& profile = m_Profiles[r]; + profile.connectGoodCount += 1; + profile.lastUpdated = llarp::time_now_ms(); } void @@ -186,24 +187,27 @@ namespace llarp Profiling::MarkHopFail(const RouterID& r) { util::Lock lock{m_ProfilesMutex}; - m_Profiles[r].pathFailCount += 1; - m_Profiles[r].lastUpdated = llarp::time_now_ms(); + auto& profile = m_Profiles[r]; + profile.pathFailCount += 1; + profile.lastUpdated = llarp::time_now_ms(); } void Profiling::MarkPathFail(path::Path* p) { util::Lock lock{m_ProfilesMutex}; - size_t idx = 0; + bool first = true; for (const auto& hop : p->hops) { // don't mark first hop as failure because we are connected to it directly - if (idx) + if (first) + first = false; + else { - m_Profiles[hop.rc.pubkey].pathFailCount += 1; - m_Profiles[hop.rc.pubkey].lastUpdated = llarp::time_now_ms(); + auto& profile = m_Profiles[hop.rc.pubkey]; + profile.pathFailCount += 1; + profile.lastUpdated = llarp::time_now_ms(); } - ++idx; } } @@ -213,8 +217,9 @@ namespace llarp util::Lock lock{m_ProfilesMutex}; for (const auto& hop : p->hops) { - m_Profiles[hop.rc.pubkey].pathTimeoutCount += 1; - m_Profiles[hop.rc.pubkey].lastUpdated = llarp::time_now_ms(); + auto& profile = m_Profiles[hop.rc.pubkey]; + profile.pathTimeoutCount += 1; + profile.lastUpdated = llarp::time_now_ms(); } } @@ -225,88 +230,82 @@ namespace llarp const auto sz = p->hops.size(); for (const auto& hop : p->hops) { + auto& profile = m_Profiles[hop.rc.pubkey]; // redeem previous fails by halfing the fail count and setting timeout to zero - m_Profiles[hop.rc.pubkey].pathFailCount /= 2; - m_Profiles[hop.rc.pubkey].pathTimeoutCount = 0; + profile.pathFailCount /= 2; + profile.pathTimeoutCount = 0; // mark success at hop - m_Profiles[hop.rc.pubkey].pathSuccessCount += sz; - m_Profiles[hop.rc.pubkey].lastUpdated = llarp::time_now_ms(); + profile.pathSuccessCount += sz; + profile.lastUpdated = llarp::time_now_ms(); } } bool Profiling::Save(const fs::path fpath) { - const size_t sz = (m_Profiles.size() * (RouterProfile::MaxSize + 32 + 8)) + 8; - - std::vector tmp(sz, 0); - llarp_buffer_t buf(tmp); - + std::string buf; { util::Lock lock{m_ProfilesMutex}; - if (not BEncode(&buf)) + buf.resize((m_Profiles.size() * (RouterProfile::MaxSize + 32 + 8)) + 8); + bt_dict_producer d{buf.data(), buf.size()}; + try + { + BEncode(d); + } + catch (const std::exception& e) + { + log::warning(logcat, "Failed to encode profiling data: {}", e.what()); return false; + } + buf.resize(d.end() - buf.data()); } - buf.sz = buf.cur - buf.base; - auto optional_f = util::OpenFileStream(fpath, std::ios::binary); - if (!optional_f) - return false; - auto& f = *optional_f; - if (not f.is_open()) + try + { + util::dump_file(fpath, buf); + } + catch (const std::exception& e) + { + log::warning(logcat, "Failed to save profiling data to {}: {}", fpath, e.what()); return false; + } - f.write(reinterpret_cast(buf.base), buf.sz); - if (not f.good()) - return false; m_LastSave = llarp::time_now_ms(); return true; } - bool - Profiling::BEncode(llarp_buffer_t* buf) const - { - if (!bencode_start_dict(buf)) - return false; - - auto itr = m_Profiles.begin(); - while (itr != m_Profiles.end()) - { - if (!itr->first.BEncode(buf)) - return false; - if (!itr->second.BEncode(buf)) - return false; - ++itr; - } - return bencode_end(buf); - } - - bool - Profiling::DecodeKey(const llarp_buffer_t& k, llarp_buffer_t* buf) + void + Profiling::BEncode(bt_dict_producer& dict) const { - if (k.sz != 32) - return false; - RouterProfile profile; - if (!bencode_decode_dict(profile, buf)) - return false; - RouterID pk = k.base; - return m_Profiles.emplace(pk, profile).second; + for (const auto& [r_id, profile] : m_Profiles) + profile.BEncode(dict.append_dict(r_id.ToView())); } - bool - Profiling::BDecode(llarp_buffer_t* buf) + void + Profiling::BDecode(bt_dict_consumer dict) { - return bencode_decode_dict(*this, buf); + m_Profiles.clear(); + while (dict) + { + auto [rid, subdict] = dict.next_dict_consumer(); + if (rid.size() != RouterID::SIZE) + throw std::invalid_argument{"invalid RouterID"}; + m_Profiles.emplace(reinterpret_cast(rid.data()), subdict); + } } bool Profiling::Load(const fs::path fname) { - util::Lock lock{m_ProfilesMutex}; - m_Profiles.clear(); - if (!BDecodeReadFile(fname, *this)) + try + { + std::string data = util::slurp_file(fname); + util::Lock lock{m_ProfilesMutex}; + BDecode(bt_dict_consumer{data}); + } + catch (const std::exception& e) { - llarp::LogWarn("failed to load router profiles from ", fname); + log::warning(logcat, "failed to load router profiles from {}: {}", fname, e.what()); return false; } m_LastSave = llarp::time_now_ms(); diff --git a/llarp/profiling.hpp b/llarp/profiling.hpp index df26b6bcb..c23199b32 100644 --- a/llarp/profiling.hpp +++ b/llarp/profiling.hpp @@ -8,6 +8,12 @@ #include "util/thread/annotations.hpp" #include +namespace oxenc +{ + class bt_dict_consumer; + class bt_dict_producer; +} // namespace oxenc + namespace llarp { struct RouterProfile @@ -22,11 +28,19 @@ namespace llarp llarp_time_t lastDecay = 0s; uint64_t version = llarp::constants::proto_version; - bool - BEncode(llarp_buffer_t* buf) const; + RouterProfile() = default; + RouterProfile(oxenc::bt_dict_consumer dict); - bool - DecodeKey(const llarp_buffer_t& k, llarp_buffer_t* buf); + void + BEncode(oxenc::bt_dict_producer& dict) const; + void + BEncode(oxenc::bt_dict_producer&& dict) const + { + BEncode(dict); + } + + void + BDecode(oxenc::bt_dict_consumer dict); bool IsGood(uint64_t chances) const; @@ -89,16 +103,6 @@ namespace llarp void Tick() EXCLUDES(m_ProfilesMutex); - bool - BEncode(llarp_buffer_t* buf) const; - - bool - BDecode(llarp_buffer_t* buf); - - bool - DecodeKey(const llarp_buffer_t& k, llarp_buffer_t* buf) NO_THREAD_SAFETY_ANALYSIS; - // disabled because we do load -> bencode::BDecodeReadFromFile -> DecodeKey - bool Load(const fs::path fname) EXCLUDES(m_ProfilesMutex); @@ -115,6 +119,12 @@ namespace llarp Enable(); private: + void + BEncode(oxenc::bt_dict_producer& dict) const; + + void + BDecode(oxenc::bt_dict_consumer dict); + mutable util::Mutex m_ProfilesMutex; // protects m_Profiles std::map m_Profiles GUARDED_BY(m_ProfilesMutex); llarp_time_t m_LastSave = 0s; diff --git a/llarp/util/aligned.hpp b/llarp/util/aligned.hpp index 92dcf38a4..fb756d562 100644 --- a/llarp/util/aligned.hpp +++ b/llarp/util/aligned.hpp @@ -260,6 +260,12 @@ namespace llarp return FromBytestring(&strbuf); } + std::string_view + ToView() const + { + return {reinterpret_cast(data()), sz}; + } + std::string ToHex() const {