From dcec5a0bbcc96e8df1f9591ffe2f41cca91dd23f Mon Sep 17 00:00:00 2001 From: JCW Date: Mon, 8 Sep 2025 16:40:34 +0100 Subject: [PATCH] Revert unrelated changes & performance optimisation Signed-off-by: JCW --- include/xrpl/basics/Log.h | 8 +- include/xrpl/beast/utility/Journal.h | 6 +- include/xrpl/beast/utility/WrappedSink.h | 8 +- include/xrpl/resource/detail/Logic.h | 29 ++----- src/libxrpl/basics/Log.cpp | 91 +++++++++++---------- src/libxrpl/beast/utility/beast_Journal.cpp | 10 +-- src/test/beast/beast_Journal_test.cpp | 4 +- src/test/csf/Sim.h | 4 +- src/test/jtx/CaptureLogs.h | 4 +- src/test/jtx/CheckMessageLogs.h | 4 +- src/test/server/Server_test.cpp | 4 +- src/test/unit_test/SuiteJournal.h | 12 +-- src/tests/libxrpl/basics/log.cpp | 10 +-- 13 files changed, 93 insertions(+), 101 deletions(-) diff --git a/include/xrpl/basics/Log.h b/include/xrpl/basics/Log.h index c2a66efb79..dae3fa14af 100644 --- a/include/xrpl/basics/Log.h +++ b/include/xrpl/basics/Log.h @@ -71,10 +71,10 @@ private: operator=(Sink const&) = delete; void - write(beast::severities::Severity level, std::string&& text) override; + write(beast::severities::Severity level, std::string_view text) override; void - writeAlways(beast::severities::Severity level, std::string&& text) + writeAlways(beast::severities::Severity level, std::string_view text) override; }; @@ -205,7 +205,7 @@ public: write( beast::severities::Severity level, std::string const& partition, - std::string const& text, + std::string_view text, bool console); std::string @@ -246,7 +246,7 @@ public: static void format( std::string& output, - std::string const& message, + std::string_view message, beast::severities::Severity severity, std::string const& partition); diff --git a/include/xrpl/beast/utility/Journal.h b/include/xrpl/beast/utility/Journal.h index 0e02336033..da73a0a3e0 100644 --- a/include/xrpl/beast/utility/Journal.h +++ b/include/xrpl/beast/utility/Journal.h @@ -366,7 +366,7 @@ private: severities::Severity severity) const; static std::string_view - formatLog(std::string&& message); + formatLog(std::string const& message); public: //-------------------------------------------------------------------------- @@ -420,7 +420,7 @@ public: level is below the current threshold(). */ virtual void - write(Severity level, std::string&& text) = 0; + write(Severity level, std::string_view text) = 0; /** Bypass filter and write text to the sink at the specified severity. * Always write the message, but maintain the same formatting as if @@ -430,7 +430,7 @@ public: * @param text Text to write to sink. */ virtual void - writeAlways(Severity level, std::string&& text) = 0; + writeAlways(Severity level, std::string_view text) = 0; private: Severity thresh_; diff --git a/include/xrpl/beast/utility/WrappedSink.h b/include/xrpl/beast/utility/WrappedSink.h index ee8641849d..e1343dc9e6 100644 --- a/include/xrpl/beast/utility/WrappedSink.h +++ b/include/xrpl/beast/utility/WrappedSink.h @@ -88,17 +88,17 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { using beast::Journal; - sink_.write(level, prefix_ + text); + sink_.write(level, prefix_ + std::string{text}); } void - writeAlways(severities::Severity level, std::string&& text) override + writeAlways(severities::Severity level, std::string_view text) override { using beast::Journal; - sink_.writeAlways(level, prefix_ + text); + sink_.writeAlways(level, prefix_ + std::string{text}); } }; diff --git a/include/xrpl/resource/detail/Logic.h b/include/xrpl/resource/detail/Logic.h index 8bfa2c2ac8..b07ee00e73 100644 --- a/include/xrpl/resource/detail/Logic.h +++ b/include/xrpl/resource/detail/Logic.h @@ -132,8 +132,7 @@ public: } } - JLOG(m_journal.debug()) - << "New inbound endpoint " << log::param("Entry", *entry); + JLOG(m_journal.debug()) << "New inbound endpoint " << *entry; return Consumer(*this, *entry); } @@ -161,8 +160,7 @@ public: } } - JLOG(m_journal.debug()) - << "New outbound endpoint " << log::param("Entry", *entry); + JLOG(m_journal.debug()) << "New outbound endpoint " << *entry; return Consumer(*this, *entry); } @@ -195,8 +193,7 @@ public: } } - JLOG(m_journal.debug()) - << "New unlimited endpoint " << log::param("Entry", *entry); + JLOG(m_journal.debug()) << "New unlimited endpoint " << *entry; return Consumer(*this, *entry); } @@ -353,8 +350,7 @@ public: { if (iter->whenExpires <= elapsed) { - JLOG(m_journal.debug()) - << "Expired " << log::param("Entry", *iter); + JLOG(m_journal.debug()) << "Expired " << *iter; auto table_iter = table_.find(*iter->key); ++iter; erase(table_iter); @@ -426,9 +422,7 @@ public: std::lock_guard _(lock_); if (--entry.refcount == 0) { - JLOG(m_journal.debug()) - << "Inactive " << log::param("Entry", entry); - ; + JLOG(m_journal.debug()) << "Inactive " << entry; switch (entry.key->kind) { @@ -480,8 +474,7 @@ public: clock_type::time_point const now(m_clock.now()); int const balance(entry.add(fee.cost(), now)); JLOG(getStream(fee.cost(), m_journal)) - << "Charging " << log::param("Entry", entry) << " for " - << log::param("Fee", fee) << context; + << "Charging " << entry << " for " << fee << context; return disposition(balance); } @@ -503,9 +496,7 @@ public: } if (notify) { - JLOG(m_journal.info()) - << "Load warning: " << log::param("Entry", entry); - ; + JLOG(m_journal.info()) << "Load warning: " << entry; ++m_stats.warn; } return notify; @@ -524,10 +515,8 @@ public: if (balance >= dropThreshold) { JLOG(m_journal.warn()) - << "Consumer entry " << log::param("Entry", entry) - << " dropped with balance " << log::param("Entry", balance) - << " at or above drop threshold " - << log::param("Entry", dropThreshold); + << "Consumer entry " << entry << " dropped with balance " + << balance << " at or above drop threshold " << dropThreshold; // Adding feeDrop at this point keeps the dropped connection // from re-connecting for at least a little while after it is diff --git a/src/libxrpl/basics/Log.cpp b/src/libxrpl/basics/Log.cpp index 996c8bf3f1..cdb4b5820b 100644 --- a/src/libxrpl/basics/Log.cpp +++ b/src/libxrpl/basics/Log.cpp @@ -52,18 +52,18 @@ Logs::Sink::Sink( } void -Logs::Sink::write(beast::severities::Severity level, std::string&& text) +Logs::Sink::write(beast::severities::Severity level, std::string_view text) { if (level < threshold()) return; - logs_.write(level, partition_, std::move(text), console()); + logs_.write(level, partition_, text, console()); } void -Logs::Sink::writeAlways(beast::severities::Severity level, std::string&& text) +Logs::Sink::writeAlways(beast::severities::Severity level, std::string_view text) { - logs_.write(level, partition_, std::move(text), console()); + logs_.write(level, partition_, text, console()); } //------------------------------------------------------------------------------ @@ -190,21 +190,26 @@ void Logs::write( beast::severities::Severity level, std::string const& partition, - std::string const& text, + std::string_view text, bool console) { std::string s; - format(s, text, level, partition); + std::string_view result = text; + if (!beast::Journal::isStructuredJournalEnabled()) + { + format(s, text, level, partition); + result = s; + } // Console output still immediate for responsiveness if (!silent_) - std::cerr << s << '\n'; + std::cerr << result << '\n'; // Add to batch buffer for file output { std::lock_guard lock(batchMutex_); - size_t logSize = s.size() + 1; // +1 for newline + size_t logSize = result.size() + 1; // +1 for newline // If log won't fit in current write buffer, flush first if (logSize > writeBuffer_.size()) @@ -213,8 +218,8 @@ Logs::write( } // Copy log into write buffer - std::copy(s.begin(), s.end(), writeBuffer_.begin()); - writeBuffer_[s.size()] = '\n'; + std::copy(result.begin(), result.end(), writeBuffer_.begin()); + writeBuffer_[result.size()] = '\n'; // Update spans: expand read buffer, shrink write buffer size_t totalUsed = readBuffer_.size() + logSize; @@ -378,47 +383,45 @@ Logs::fromString(std::string const& s) void Logs::format( std::string& output, - std::string const& message, + std::string_view message, beast::severities::Severity severity, std::string const& partition) { output = message; - if (!beast::Journal::isStructuredJournalEnabled()) + output.reserve(output.size() + partition.size() + 100); + output += to_string(std::chrono::system_clock::now()); + + output += " "; + if (!partition.empty()) + output += partition + ":"; + + using namespace beast::severities; + switch (severity) { - output.reserve(output.size() + partition.size() + 100); - output += to_string(std::chrono::system_clock::now()); - - output += " "; - if (!partition.empty()) - output += partition + ":"; - - using namespace beast::severities; - switch (severity) - { - case kTrace: - output += "TRC "; - break; - case kDebug: - output += "DBG "; - break; - case kInfo: - output += "NFO "; - break; - case kWarning: - output += "WRN "; - break; - case kError: - output += "ERR "; - break; - default: - UNREACHABLE("ripple::Logs::format : invalid severity"); - [[fallthrough]]; - case kFatal: - output += "FTL "; - break; - } + case kTrace: + output += "TRC "; + break; + case kDebug: + output += "DBG "; + break; + case kInfo: + output += "NFO "; + break; + case kWarning: + output += "WRN "; + break; + case kError: + output += "ERR "; + break; + default: + UNREACHABLE("ripple::Logs::format : invalid severity"); + [[fallthrough]]; + case kFatal: + output += "FTL "; + break; } + // Limit the maximum length of the output if (output.size() > maximumMessageCharacters) { diff --git a/src/libxrpl/beast/utility/beast_Journal.cpp b/src/libxrpl/beast/utility/beast_Journal.cpp index 88ada9664b..8c4663ad29 100644 --- a/src/libxrpl/beast/utility/beast_Journal.cpp +++ b/src/libxrpl/beast/utility/beast_Journal.cpp @@ -156,12 +156,12 @@ public: } void - write(severities::Severity, std::string&&) override + write(severities::Severity, std::string_view) override { } void - writeAlways(severities::Severity, std::string&&) override + writeAlways(severities::Severity, std::string_view) override { } }; @@ -293,7 +293,7 @@ Journal::initMessageContext( } std::string_view -Journal::formatLog(std::string&& message) +Journal::formatLog(std::string const& message) { if (!m_jsonLogsEnabled) { @@ -391,9 +391,9 @@ Journal::ScopedStream::~ScopedStream() if (!s.empty()) { if (s == "\n") - m_sink.write(m_level, std::string{formatLog("")}); + m_sink.write(m_level, formatLog("")); else - m_sink.write(m_level, std::string{formatLog(std::move(s))}); + m_sink.write(m_level, formatLog(s)); } } diff --git a/src/test/beast/beast_Journal_test.cpp b/src/test/beast/beast_Journal_test.cpp index abd4af4f8d..b1379a3456 100644 --- a/src/test/beast/beast_Journal_test.cpp +++ b/src/test/beast/beast_Journal_test.cpp @@ -48,14 +48,14 @@ public: } void - write(severities::Severity level, std::string&&) override + write(severities::Severity level, std::string_view) override { if (level >= threshold()) ++m_count; } void - writeAlways(severities::Severity level, std::string&&) override + writeAlways(severities::Severity level, std::string_view) override { ++m_count; } diff --git a/src/test/csf/Sim.h b/src/test/csf/Sim.h index b61cbbeb1a..313f8abd8f 100644 --- a/src/test/csf/Sim.h +++ b/src/test/csf/Sim.h @@ -49,7 +49,7 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { if (level < threshold()) return; @@ -59,7 +59,7 @@ public: } void - writeAlways(beast::severities::Severity level, std::string&& text) override + writeAlways(beast::severities::Severity level, std::string_view text) override { std::cout << clock_.now().time_since_epoch().count() << " " << text << std::endl; diff --git a/src/test/jtx/CaptureLogs.h b/src/test/jtx/CaptureLogs.h index c89f1f74fa..4fffae130e 100644 --- a/src/test/jtx/CaptureLogs.h +++ b/src/test/jtx/CaptureLogs.h @@ -57,14 +57,14 @@ class CaptureLogs : public Logs } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { std::lock_guard lock(strmMutex_); strm_ << text; } void - writeAlways(beast::severities::Severity level, std::string&& text) + writeAlways(beast::severities::Severity level, std::string_view text) override { std::lock_guard lock(strmMutex_); diff --git a/src/test/jtx/CheckMessageLogs.h b/src/test/jtx/CheckMessageLogs.h index e05ac5e4cf..6b950a82ee 100644 --- a/src/test/jtx/CheckMessageLogs.h +++ b/src/test/jtx/CheckMessageLogs.h @@ -45,14 +45,14 @@ class CheckMessageLogs : public Logs } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { if (text.find(owner_.msg_) != std::string::npos) *owner_.pFound_ = true; } void - writeAlways(beast::severities::Severity level, std::string&& text) + writeAlways(beast::severities::Severity level, std::string_view text) override { write(level, std::move(text)); diff --git a/src/test/server/Server_test.cpp b/src/test/server/Server_test.cpp index db1c58c2bc..1c5aa67992 100644 --- a/src/test/server/Server_test.cpp +++ b/src/test/server/Server_test.cpp @@ -92,7 +92,7 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { if (level < threshold()) return; @@ -101,7 +101,7 @@ public: } void - writeAlways(beast::severities::Severity level, std::string&& text) + writeAlways(beast::severities::Severity level, std::string_view text) override { suite_.log << text << std::endl; diff --git a/src/test/unit_test/SuiteJournal.h b/src/test/unit_test/SuiteJournal.h index 8fb0f122bd..e58e435e6f 100644 --- a/src/test/unit_test/SuiteJournal.h +++ b/src/test/unit_test/SuiteJournal.h @@ -49,14 +49,14 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override; + write(beast::severities::Severity level, std::string_view text) override; void - writeAlways(beast::severities::Severity level, std::string&& text) override; + writeAlways(beast::severities::Severity level, std::string_view text) override; }; inline void -SuiteJournalSink::write(beast::severities::Severity level, std::string&& text) +SuiteJournalSink::write(beast::severities::Severity level, std::string_view text) { // Only write the string if the level at least equals the threshold. if (level >= threshold()) @@ -66,7 +66,7 @@ SuiteJournalSink::write(beast::severities::Severity level, std::string&& text) inline void SuiteJournalSink::writeAlways( beast::severities::Severity level, - std::string&& text) + std::string_view text) { using namespace beast::severities; @@ -134,7 +134,7 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { if (level < threshold()) return; @@ -142,7 +142,7 @@ public: } inline void - writeAlways(beast::severities::Severity level, std::string&& text) override + writeAlways(beast::severities::Severity level, std::string_view text) override { strm_ << text << std::endl; } diff --git a/src/tests/libxrpl/basics/log.cpp b/src/tests/libxrpl/basics/log.cpp index fe631b4100..4ad24a896d 100644 --- a/src/tests/libxrpl/basics/log.cpp +++ b/src/tests/libxrpl/basics/log.cpp @@ -53,13 +53,13 @@ private: operator=(Sink const&) = delete; void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { logs_.write(level, partition_, text, false); } void - writeAlways(beast::severities::Severity level, std::string&& text) + writeAlways(beast::severities::Severity level, std::string_view text) override { logs_.write(level, partition_, text, false); @@ -86,7 +86,7 @@ public: write( beast::severities::Severity level, std::string const& partition, - std::string const& text, + std::string_view text, bool console) { std::string s; @@ -365,13 +365,13 @@ public: } void - write(beast::severities::Severity level, std::string&& text) override + write(beast::severities::Severity level, std::string_view text) override { strm_ << text; } void - writeAlways(beast::severities::Severity level, std::string&& text) override + writeAlways(beast::severities::Severity level, std::string_view text) override { strm_ << text; }