From 6a2a4c303223255f698048b91aa6b7ccd486b86b Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 4 Jul 2023 14:03:45 -0500 Subject: [PATCH] perf: move log strings instead of cloning them (#5726) --- libtransmission/announcer-udp.cc | 4 ++-- libtransmission/announcer.cc | 12 ++++++------ libtransmission/file.cc | 10 ++-------- libtransmission/inout.cc | 4 ++-- libtransmission/log.cc | 27 +++++++++++++++++---------- libtransmission/log.h | 2 +- libtransmission/torrent.cc | 6 ++++-- 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/libtransmission/announcer-udp.cc b/libtransmission/announcer-udp.cc index b73970204..c4b2ae032 100644 --- a/libtransmission/announcer-udp.cc +++ b/libtransmission/announcer-udp.cc @@ -324,9 +324,9 @@ struct tau_tracker } else if (action == TAU_ACTION_ERROR) { - std::string const errmsg = !std::empty(buf) ? buf.to_string() : _("Connection failed"); - logdbg(this->key, errmsg); + std::string errmsg = !std::empty(buf) ? buf.to_string() : _("Connection failed"); this->failAll(true, false, errmsg); + logdbg(this->key, std::move(errmsg)); } this->upkeep(); diff --git a/libtransmission/announcer.cc b/libtransmission/announcer.cc index 767d4b76e..05217cb5e 100644 --- a/libtransmission/announcer.cc +++ b/libtransmission/announcer.cc @@ -771,7 +771,7 @@ void tr_logAddTrace_tier_announce_queue(tr_tier const* tier) fmt::format_to(std::back_inserter(buf), FMT_STRING("[{:d}:{:s}]"), i, tr_announce_event_get_string(events[i])); } - tr_logAddTraceTier(tier, buf); + tr_logAddTraceTier(tier, std::move(buf)); } // higher priorities go to the front of the announce queue @@ -876,7 +876,7 @@ void on_announce_error(tr_tier* tier, char const* err, tr_announce_event e) { tr_logAddErrorTier( tier, - fmt::format(_("Announce error: {error}"), fmt::arg("error", err)).append(fmt::format(" ({})", announce_url))); + fmt::format(_("Announce error: {error} ({url})"), fmt::arg("error", err), fmt::arg("url", announce_url))); } else { @@ -886,12 +886,12 @@ void on_announce_error(tr_tier* tier, char const* err, tr_announce_event e) tier, fmt::format( tr_ngettext( - "Announce error: {error} (Retrying in {count} second)", - "Announce error: {error} (Retrying in {count} seconds)", + "Announce error: {error} (Retrying in {count} second) ({url})", + "Announce error: {error} (Retrying in {count} seconds) ({url})", interval), fmt::arg("error", err), - fmt::arg("count", interval)) - .append(fmt::format(" ({})", announce_url))); + fmt::arg("count", interval), + fmt::arg("url", announce_url))); tier_announce_event_push(tier, e, tr_time() + interval); } } diff --git a/libtransmission/file.cc b/libtransmission/file.cc index ad6292acf..b672c07c3 100644 --- a/libtransmission/file.cc +++ b/libtransmission/file.cc @@ -26,14 +26,8 @@ bool tr_sys_file_write_line(tr_sys_file_t handle, std::string_view buffer, tr_er { TR_ASSERT(handle != TR_BAD_SYS_FILE); - bool ret = tr_sys_file_write(handle, std::data(buffer), std::size(buffer), nullptr, error); - - if (ret) - { - ret = tr_sys_file_write(handle, std::data(NativeEol), std::size(NativeEol), nullptr, error); - } - - return ret; + return tr_sys_file_write(handle, std::data(buffer), std::size(buffer), nullptr, error) && + tr_sys_file_write(handle, std::data(NativeEol), std::size(NativeEol), nullptr, error); } std::vector tr_sys_dir_get_files( diff --git a/libtransmission/inout.cc b/libtransmission/inout.cc index f536ac0d9..2c5f7d619 100644 --- a/libtransmission/inout.cc +++ b/libtransmission/inout.cc @@ -147,13 +147,13 @@ void readOrWriteBytes( if (!fd) // couldn't create/open it either { auto const err = errno; - auto const msg = fmt::format( + auto msg = fmt::format( _("Couldn't get '{path}': {error} ({error_code})"), fmt::arg("path", filename), fmt::arg("error", tr_strerror(err)), fmt::arg("error_code", err)); tr_error_set(error, err, msg); - tr_logAddErrorTor(tor, msg); + tr_logAddErrorTor(tor, std::move(msg)); return; } diff --git a/libtransmission/log.cc b/libtransmission/log.cc index 366212b35..6ee0e7895 100644 --- a/libtransmission/log.cc +++ b/libtransmission/log.cc @@ -25,6 +25,7 @@ #include "libtransmission/file.h" #include "libtransmission/log.h" #include "libtransmission/tr-assert.h" +#include "libtransmission/tr-strbuf.h" #include "libtransmission/utils.h" using namespace std::literals; @@ -61,7 +62,7 @@ void logAddImpl( [[maybe_unused]] std::string_view file, [[maybe_unused]] long line, [[maybe_unused]] tr_log_level level, - std::string_view msg, + std::string&& msg, [[maybe_unused]] std::string_view name) { if (std::empty(msg)) @@ -110,7 +111,7 @@ void logAddImpl( auto* const newmsg = new tr_log_message{}; newmsg->level = level; newmsg->when = tr_time(); - newmsg->message = msg; + newmsg->message = std::move(msg); newmsg->file = file; newmsg->line = line; newmsg->name = name; @@ -132,18 +133,24 @@ void logAddImpl( else { static auto const fp = tr_sys_file_get_std(TR_STD_SYS_FILE_ERR); - if (fp == TR_BAD_SYS_FILE) { return; } - auto timestr = std::array{}; + auto timestr = std::array{}; tr_logGetTimeStr(std::data(timestr), std::size(timestr)); - tr_sys_file_write_line( - fp, - !std::empty(name) ? fmt::format(FMT_STRING("[{:s}] {:s}: {:s}"), std::data(timestr), name, msg) : - fmt::format(FMT_STRING("[{:s}] {:s}"), std::data(timestr), msg)); + + auto buf = tr_strbuf{}; + if (std::empty(name)) + { + fmt::format_to(std::back_inserter(buf), "[{:s}] {:s}", std::data(timestr), msg); + } + else + { + fmt::format_to(std::back_inserter(buf), "[{:s}] {:s}: {:s}", std::data(timestr), name, msg); + } + tr_sys_file_write_line(fp, buf); tr_sys_file_flush(fp); } #endif @@ -208,7 +215,7 @@ char* tr_logGetTimeStr(char* buf, size_t buflen) return buf; } -void tr_logAddMessage(char const* file, long line, tr_log_level level, std::string_view msg, std::string_view name) +void tr_logAddMessage(char const* file, long line, tr_log_level level, std::string&& msg, std::string_view name) { TR_ASSERT(!std::empty(msg)); @@ -257,7 +264,7 @@ void tr_logAddMessage(char const* file, long line, tr_log_level level, std::stri } // log the messages - logAddImpl(filename, line, level, msg, name); + logAddImpl(filename, line, level, std::move(msg), name); if (last_one) { logAddImpl( diff --git a/libtransmission/log.h b/libtransmission/log.h index 59c9530a6..299188209 100644 --- a/libtransmission/log.h +++ b/libtransmission/log.h @@ -89,7 +89,7 @@ void tr_logAddMessage( char const* source_file, long source_line, tr_log_level level, - std::string_view msg, + std::string&& msg, std::string_view module_name = {}); #define tr_logAddLevel(level, ...) \ diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 890f8ef28..00e5b3f66 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -2176,8 +2176,10 @@ void tr_torrent::on_tracker_response(tr_tracker_event const* event) case tr_tracker_event::Type::Warning: tr_logAddWarnTor( this, - fmt::format(_("Tracker warning: '{warning}'"), fmt::arg("warning", event->text)) - .append(fmt::format(" ({})", tr_urlTrackerLogName(event->announce_url)))); + fmt::format( + _("Tracker warning: '{warning}' ({url})"), + fmt::arg("warning", event->text), + fmt::arg("url", tr_urlTrackerLogName(event->announce_url)))); error = TR_STAT_TRACKER_WARNING; error_announce_url = event->announce_url; error_string = event->text;