From 197c1adcbab034584a443742d3f2a56beae9e4d5 Mon Sep 17 00:00:00 2001 From: Levi Behunin Date: Sat, 24 Jun 2023 11:41:53 +0300 Subject: [PATCH] Refactor Logging Impl Loop on stop_token and remove final_entry in Entry. Move Backend thread out of Impl Constructor to its own function. Add Start function for backend thread. Use stop token in PopWait and check if entry filename is nullptr before logging. --- src/citra/citra.cpp | 1 + src/citra_qt/main.cpp | 2 ++ src/common/logging/backend.cpp | 65 +++++++++++++++++++--------------- src/common/logging/backend.h | 2 ++ src/common/logging/log_entry.h | 1 - 5 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index aade3ee9c9..e3b71d26db 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -178,6 +178,7 @@ static void OnStatusMessageReceived(const Network::StatusMessageEntry& msg) { int main(int argc, char** argv) { Common::Log::Initialize(); Common::Log::SetColorConsoleBackendEnabled(true); + Common::Log::Start(); Common::DetachedTasks detached_tasks; Config config; int option_index = 0; diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index ee1102a218..cc76ecd8e2 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -264,6 +264,8 @@ GMainWindow::GMainWindow(Core::System& system_) } #endif + Common::Log::Start(); + QStringList args = QApplication::arguments(); if (args.length() >= 2) { BootGame(args[1]); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 19efa5dcb4..ad01c24374 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include @@ -31,6 +30,7 @@ #include "common/logging/log.h" #include "common/logging/log_entry.h" #include "common/logging/text_formatter.h" +#include "common/polyfill_thread.h" #include "common/settings.h" #include "common/string_util.h" #include "common/thread.h" @@ -219,6 +219,10 @@ public: initialization_in_progress_suppress_logging = false; } + static void Start() { + instance->StartBackendThread(); + } + Impl(const Impl&) = delete; Impl& operator=(const Impl&) = delete; @@ -244,26 +248,7 @@ public: private: Impl(const std::string& file_backend_filename, const Filter& filter_) - : filter{filter_}, file_backend{file_backend_filename}, backend_thread{std::thread([this] { - Common::SetCurrentThreadName("citra:Log"); - Entry entry; - const auto write_logs = [this, &entry]() { - ForEachBackend([&entry](Backend& backend) { backend.Write(entry); }); - }; - while (true) { - entry = message_queue.PopWait(); - if (entry.final_entry) { - break; - } - write_logs(); - } - // Drain the logging queue. Only writes out up to MAX_LOGS_TO_WRITE to prevent a - // case where a system is repeatedly spamming logs even on close. - int max_logs_to_write = filter.IsDebug() ? INT_MAX : 100; - while (max_logs_to_write-- && message_queue.Pop(entry)) { - write_logs(); - } - })} { + : filter{filter_}, file_backend{file_backend_filename} { #ifdef CITRA_LINUX_GCC_BACKTRACE int waker_pipefd[2]; int done_printing_pipefd[2]; @@ -297,7 +282,7 @@ private: } line.pop_back(); // Remove newline const auto frame_entry = - CreateEntry(Class::Log, Level::Critical, "?", 0, "?", line); + CreateEntry(Class::Log, Level::Critical, "?", 0, "?", std::move(line)); ForEachBackend([&frame_entry](Backend& backend) { backend.Write(frame_entry); }); } using namespace std::literals; @@ -326,15 +311,35 @@ private: StopBackendThread(); } + void StartBackendThread() { + backend_thread = std::thread([this] { + Common::SetCurrentThreadName("citra:Log"); + Entry entry; + const auto write_logs = [this, &entry]() { + ForEachBackend([&entry](Backend& backend) { backend.Write(entry); }); + }; + while (!stop.stop_requested()) { + entry = message_queue.PopWait(stop.get_token()); + if (entry.filename != nullptr) { + write_logs(); + } + } + // Drain the logging queue. Only writes out up to MAX_LOGS_TO_WRITE to prevent a + // case where a system is repeatedly spamming logs even on close. + int max_logs_to_write = filter.IsDebug() ? INT_MAX : 100; + while (max_logs_to_write-- && message_queue.Pop(entry)) { + write_logs(); + } + }); + } + void StopBackendThread() { - Entry stop_entry{}; - stop_entry.final_entry = true; - message_queue.Push(stop_entry); + stop.request_stop(); backend_thread.join(); } Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, std::string message) const { + const char* function, std::string&& message) const { using std::chrono::duration_cast; using std::chrono::microseconds; using std::chrono::steady_clock; @@ -347,7 +352,6 @@ private: .line_num = line_nr, .function = function, .message = std::move(message), - .final_entry = false, }; } @@ -398,8 +402,9 @@ private: ColorConsoleBackend color_console_backend{}; FileBackend file_backend; + std::stop_source stop; std::thread backend_thread; - MPSCQueue message_queue{}; + MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; #ifdef CITRA_LINUX_GCC_BACKTRACE @@ -415,6 +420,10 @@ void Initialize() { Impl::Initialize(); } +void Start() { + Impl::Start(); +} + void DisableLoggingInTests() { initialization_in_progress_suppress_logging = true; } diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index cb7839ee93..bf785f402e 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -14,6 +14,8 @@ class Filter; /// Initializes the logging system. This should be the first thing called in main. void Initialize(); +void Start(); + void DisableLoggingInTests(); /** diff --git a/src/common/logging/log_entry.h b/src/common/logging/log_entry.h index dd6f448418..b28570071c 100644 --- a/src/common/logging/log_entry.h +++ b/src/common/logging/log_entry.h @@ -22,7 +22,6 @@ struct Entry { unsigned int line_num = 0; std::string function; std::string message; - bool final_entry = false; }; } // namespace Common::Log