From 4fd0cbebdba2446b10595ff18014ca5fddae004d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 3 Aug 2020 10:31:57 -0400 Subject: [PATCH 01/25] logging/backend: Make use of designated initializers Same behavior, less code. --- src/common/logging/backend.cpp | 22 +++++++++++----------- src/common/logging/backend.h | 14 ++++---------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index de5ce7b416..bb77991fd9 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -107,19 +107,19 @@ private: Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, const char* function, std::string message) const { using std::chrono::duration_cast; + using std::chrono::microseconds; using std::chrono::steady_clock; - Entry entry; - entry.timestamp = - duration_cast(steady_clock::now() - time_origin); - entry.log_class = log_class; - entry.log_level = log_level; - entry.filename = filename; - entry.line_num = line_nr; - entry.function = function; - entry.message = std::move(message); - - return entry; + return { + .timestamp = duration_cast(steady_clock::now() - time_origin), + .log_class = log_class, + .log_level = log_level, + .filename = filename, + .line_num = line_nr, + .function = function, + .message = std::move(message), + .final_entry = false, + }; } std::mutex writing_mutex; diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 907c6a2970..eab6db9bb1 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -20,19 +20,13 @@ namespace Log { */ struct Entry { std::chrono::microseconds timestamp; - Class log_class; - Level log_level; - const char* filename; - unsigned int line_num; + Class log_class{}; + Level log_level{}; + const char* filename = nullptr; + unsigned int line_num = 0; std::string function; std::string message; bool final_entry = false; - - Entry() = default; - Entry(Entry&& o) = default; - - Entry& operator=(Entry&& o) = default; - Entry& operator=(const Entry& o) = default; }; /** From b4c38372d1561a2a6234cac6a56eead2594b2fc8 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 24 Jun 2023 01:51:16 +0300 Subject: [PATCH 02/25] common/log: Move Log namespace into the Common namespace Forgot to move this over when I moved the rest of the source files with lacking namespaces over. --- src/citra/citra.cpp | 2 + .../configuration/configure_debug.cpp | 4 +- src/citra_qt/debugger/console.cpp | 4 +- src/citra_qt/main.cpp | 2 + src/common/logging/backend.cpp | 4 +- src/common/logging/backend.h | 4 +- src/common/logging/filter.cpp | 4 +- src/common/logging/filter.h | 2 +- src/common/logging/log.h | 38 +++++++++++-------- src/common/logging/text_formatter.cpp | 4 +- src/common/logging/text_formatter.h | 4 +- src/core/hle/kernel/process.cpp | 2 +- src/core/hle/kernel/svc.cpp | 2 +- src/core/hle/kernel/vm_manager.cpp | 4 +- src/core/hle/kernel/vm_manager.h | 2 +- src/dedicated_room/citra-room.cpp | 2 + src/video_core/renderer_opengl/gl_driver.cpp | 12 +++--- .../renderer_vulkan/vk_platform.cpp | 24 ++++++------ 18 files changed, 66 insertions(+), 54 deletions(-) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index b8ebb1ff17..7fc8188c8f 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -175,6 +175,8 @@ static void OnStatusMessageReceived(const Network::StatusMessageEntry& msg) { } static void InitializeLogging() { + using namespace Common; + Log::Filter log_filter(Log::Level::Debug); log_filter.ParseFilterString(Settings::values.log_filter.GetValue()); Log::SetGlobalFilter(log_filter); diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index a75592fe0f..5ec1414672 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -89,9 +89,9 @@ void ConfigureDebug::ApplyConfiguration() { UISettings::values.show_console = ui->toggle_console->isChecked(); Settings::values.log_filter = ui->log_filter_edit->text().toStdString(); Debugger::ToggleConsole(); - Log::Filter filter; + Common::Log::Filter filter; filter.ParseFilterString(Settings::values.log_filter.GetValue()); - Log::SetGlobalFilter(filter); + Common::Log::SetGlobalFilter(filter); Settings::values.use_cpu_jit = ui->toggle_cpu_jit->isChecked(); Settings::values.renderer_debug = ui->toggle_renderer_debug->isChecked(); diff --git a/src/citra_qt/debugger/console.cpp b/src/citra_qt/debugger/console.cpp index eecf86048d..4fb8d83dbe 100644 --- a/src/citra_qt/debugger/console.cpp +++ b/src/citra_qt/debugger/console.cpp @@ -29,13 +29,13 @@ void ToggleConsole() { freopen_s(&temp, "CONIN$", "r", stdin); freopen_s(&temp, "CONOUT$", "w", stdout); freopen_s(&temp, "CONOUT$", "w", stderr); - Log::AddBackend(std::make_unique()); + Common::Log::AddBackend(std::make_unique()); } } else { if (FreeConsole()) { // In order to close the console, we have to also detach the streams on it. // Just redirect them to NUL if there is no console window - Log::RemoveBackend(Log::ColorConsoleBackend::Name()); + Common::Log::RemoveBackend(Common::Log::ColorConsoleBackend::Name()); freopen_s(&temp, "NUL", "r", stdin); freopen_s(&temp, "NUL", "w", stdout); freopen_s(&temp, "NUL", "w", stderr); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index c00d277fd7..b69bcf3d0e 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -147,6 +147,8 @@ void GMainWindow::ShowTelemetryCallout() { const int GMainWindow::max_recent_files_item; static void InitializeLogging() { + using namespace Common; + Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter.GetValue()); Log::SetGlobalFilter(log_filter); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index bb77991fd9..cf955b0930 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -22,7 +22,7 @@ #include "common/string_util.h" #include "common/threadsafe_queue.h" -namespace Log { +namespace Common::Log { Filter filter; void SetGlobalFilter(const Filter& f) { @@ -302,4 +302,4 @@ void FmtLogMessageImpl(Class log_class, Level log_level, const char* filename, instance.PushEntry(log_class, log_level, filename, line_num, function, fmt::vformat(format, args)); } -} // namespace Log +} // namespace Common::Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index eab6db9bb1..eeb360aa49 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -12,7 +12,7 @@ #include "common/logging/filter.h" #include "common/logging/log.h" -namespace Log { +namespace Common::Log { /** * A log entry. Log entries are store in a structured format to permit more varied output @@ -143,4 +143,4 @@ const char* GetLogClassName(Class log_class); */ const char* GetLevelName(Level log_level); -} // namespace Log +} // namespace Common::Log diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index fa42b6047d..942873575f 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -7,7 +7,7 @@ #include "common/logging/filter.h" #include "common/string_util.h" -namespace Log { +namespace Common::Log { namespace { template Level GetLevelByName(const It begin, const It end) { @@ -96,4 +96,4 @@ bool Filter::CheckMessage(Class log_class, Level level) const { return static_cast(level) >= static_cast(class_levels[static_cast(log_class)]); } -} // namespace Log +} // namespace Common::Log diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index 058c7b345d..3a8f23e368 100644 --- a/src/common/logging/filter.h +++ b/src/common/logging/filter.h @@ -9,4 +9,4 @@ #include #include "common/logging/log.h" -namespace Log {} // namespace Log +namespace Common::Log {} // namespace Common::Log diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 8cd98db14c..e6a3ea5b22 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -9,7 +9,7 @@ #include "common/common_types.h" #include "common/logging/formatter.h" -namespace Log { +namespace Common::Log { // trims up to and including the last of ../, ..\, src/, src\ in a string constexpr const char* TrimSourcePath(std::string_view source) { @@ -173,33 +173,39 @@ void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsig fmt::make_format_args(args...)); } -} // namespace Log +} // namespace Common::Log // Define the fmt lib macros #define LOG_GENERIC(log_class, log_level, ...) \ - ::Log::FmtLogMessage(log_class, log_level, ::Log::TrimSourcePath(__FILE__), __LINE__, \ - __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(log_class, log_level, Common::Log::TrimSourcePath(__FILE__), \ + __LINE__, __func__, __VA_ARGS__) #ifdef _DEBUG #define LOG_TRACE(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Trace, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Trace, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) #else #define LOG_TRACE(log_class, fmt, ...) (void(0)) #endif #define LOG_DEBUG(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Debug, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Debug, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) #define LOG_INFO(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Info, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Info, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) #define LOG_WARNING(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Warning, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Warning, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) #define LOG_ERROR(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Error, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Error, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) #define LOG_CRITICAL(log_class, ...) \ - ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, \ - ::Log::TrimSourcePath(__FILE__), __LINE__, __func__, __VA_ARGS__) + Common::Log::FmtLogMessage(Common::Log::Class::log_class, Common::Log::Level::Critical, \ + Common::Log::TrimSourcePath(__FILE__), __LINE__, __func__, \ + __VA_ARGS__) diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index 7f713b1484..e66bfa43f8 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -18,7 +18,7 @@ #include "common/logging/text_formatter.h" #include "common/string_util.h" -namespace Log { +namespace Common::Log { std::string FormatLogMessage(const Entry& entry) { unsigned int time_seconds = static_cast(entry.timestamp.count() / 1000000); @@ -141,4 +141,4 @@ void PrintMessageToLogcat([[maybe_unused]] const Entry& entry) { __android_log_print(android_log_priority, "CitraNative", "%s", str.c_str()); #endif } -} // namespace Log +} // namespace Common::Log diff --git a/src/common/logging/text_formatter.h b/src/common/logging/text_formatter.h index 13430951d9..a15b41c42a 100644 --- a/src/common/logging/text_formatter.h +++ b/src/common/logging/text_formatter.h @@ -7,7 +7,7 @@ #include #include -namespace Log { +namespace Common::Log { struct Entry; @@ -19,4 +19,4 @@ void PrintMessage(const Entry& entry); void PrintColoredMessage(const Entry& entry); /// Formats and prints a log entry to the android logcat. void PrintMessageToLogcat(const Entry& entry); -} // namespace Log +} // namespace Common::Log diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index f10baa064d..2a5d152093 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -203,7 +203,7 @@ void Process::Run(s32 main_thread_priority, u32 stack_size) { status = ProcessStatus::Running; - vm_manager.LogLayout(Log::Level::Debug); + vm_manager.LogLayout(Common::Log::Level::Debug); Kernel::SetupMainThread(kernel, codeset->entrypoint, main_thread_priority, SharedFrom(this)); } diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 4c075b541e..47b02f495d 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -529,7 +529,7 @@ ResultCode SVC::ControlMemory(u32* out_addr, u32 addr0, u32 addr1, u32 size, u32 return ERR_INVALID_COMBINATION; } - process.vm_manager.LogLayout(Log::Level::Trace); + process.vm_manager.LogLayout(Common::Log::Level::Trace); return RESULT_SUCCESS; } diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 49533bb7e9..cb087cbc91 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -239,10 +239,10 @@ ResultCode VMManager::ReprotectRange(VAddr target, u32 size, VMAPermission new_p return RESULT_SUCCESS; } -void VMManager::LogLayout(Log::Level log_level) const { +void VMManager::LogLayout(Common::Log::Level log_level) const { for (const auto& p : vma_map) { const VirtualMemoryArea& vma = p.second; - LOG_GENERIC(::Log::Class::Kernel, log_level, "{:08X} - {:08X} size: {:8X} {}{}{} {}", + LOG_GENERIC(Common::Log::Class::Kernel, log_level, "{:08X} - {:08X} size: {:8X} {}{}{} {}", vma.base, vma.base + vma.size, vma.size, (u8)vma.permissions & (u8)VMAPermission::Read ? 'R' : '-', (u8)vma.permissions & (u8)VMAPermission::Write ? 'W' : '-', diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 97c601fd6e..87ecbc0375 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -202,7 +202,7 @@ public: ResultCode ReprotectRange(VAddr target, u32 size, VMAPermission new_perms); /// Dumps the address space layout to the log, for debugging - void LogLayout(Log::Level log_level) const; + void LogLayout(Common::Log::Level log_level) const; /// Gets a list of backing memory blocks for the specified range ResultVal>> GetBackingBlocksForRange(VAddr address, diff --git a/src/dedicated_room/citra-room.cpp b/src/dedicated_room/citra-room.cpp index 46072de2d0..4e68b34963 100644 --- a/src/dedicated_room/citra-room.cpp +++ b/src/dedicated_room/citra-room.cpp @@ -150,6 +150,8 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin } static void InitializeLogging(const std::string& log_file) { + using namespace Common; + Log::AddBackend(std::make_unique()); const std::string& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); diff --git a/src/video_core/renderer_opengl/gl_driver.cpp b/src/video_core/renderer_opengl/gl_driver.cpp index 9951d0ae74..c0b8c2fe4e 100644 --- a/src/video_core/renderer_opengl/gl_driver.cpp +++ b/src/video_core/renderer_opengl/gl_driver.cpp @@ -56,22 +56,22 @@ inline std::string_view GetType(GLenum type) { static void APIENTRY DebugHandler(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar* message, const void* user_param) { - Log::Level level = Log::Level::Info; + auto level = Common::Log::Level::Info; switch (severity) { case GL_DEBUG_SEVERITY_HIGH: - level = Log::Level::Critical; + level = Common::Log::Level::Critical; break; case GL_DEBUG_SEVERITY_MEDIUM: - level = Log::Level::Warning; + level = Common::Log::Level::Warning; break; case GL_DEBUG_SEVERITY_NOTIFICATION: case GL_DEBUG_SEVERITY_LOW: - level = Log::Level::Debug; + level = Common::Log::Level::Debug; break; } - LOG_GENERIC(Log::Class::Render_OpenGL, level, "{} {} {}: {}", GetSource(source), GetType(type), - id, message); + LOG_GENERIC(Common::Log::Class::Render_OpenGL, level, "{} {} {}: {}", GetSource(source), + GetType(type), id, message); } Driver::Driver(Core::TelemetrySession& telemetry_session_) diff --git a/src/video_core/renderer_vulkan/vk_platform.cpp b/src/video_core/renderer_vulkan/vk_platform.cpp index dafb86a8dc..46f44e8d47 100644 --- a/src/video_core/renderer_vulkan/vk_platform.cpp +++ b/src/video_core/renderer_vulkan/vk_platform.cpp @@ -38,23 +38,23 @@ static VKAPI_ATTR VkBool32 VKAPI_CALL DebugUtilsCallback( break; } - Log::Level level{}; + Common::Log::Level level{}; switch (severity) { case VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT: - level = Log::Level::Error; + level = Common::Log::Level::Error; break; case VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT: - level = Log::Level::Info; + level = Common::Log::Level::Info; break; case VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT: case VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT: - level = Log::Level::Debug; + level = Common::Log::Level::Debug; break; default: - level = Log::Level::Info; + level = Common::Log::Level::Info; } - LOG_GENERIC(Log::Class::Render_Vulkan, level, "{}: {}", + LOG_GENERIC(Common::Log::Class::Render_Vulkan, level, "{}: {}", callback_data->pMessageIdName ? callback_data->pMessageIdName : "", callback_data->pMessage ? callback_data->pMessage : ""); @@ -69,25 +69,25 @@ static VKAPI_ATTR VkBool32 VKAPI_CALL DebugReportCallback(VkDebugReportFlagsEXT const char* pMessage, void* pUserData) { const VkDebugReportFlagBitsEXT severity = static_cast(flags); - Log::Level level{}; + Common::Log::Level level{}; switch (severity) { case VK_DEBUG_REPORT_ERROR_BIT_EXT: - level = Log::Level::Error; + level = Common::Log::Level::Error; break; case VK_DEBUG_REPORT_INFORMATION_BIT_EXT: - level = Log::Level::Warning; + level = Common::Log::Level::Warning; break; case VK_DEBUG_REPORT_DEBUG_BIT_EXT: case VK_DEBUG_REPORT_WARNING_BIT_EXT: case VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT: - level = Log::Level::Debug; + level = Common::Log::Level::Debug; break; default: - level = Log::Level::Info; + level = Common::Log::Level::Info; } const vk::DebugReportObjectTypeEXT type = static_cast(objectType); - LOG_GENERIC(Log::Class::Render_Vulkan, level, + LOG_GENERIC(Common::Log::Class::Render_Vulkan, level, "type = {}, object = {} | MessageCode = {:#x}, LayerPrefix = {} | {}", vk::to_string(type), object, messageCode, pLayerPrefix, pMessage); From 173b84c8ffa82ae3f010e35cf9a62d8ab12086c3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 14 Apr 2021 23:05:42 -0400 Subject: [PATCH 03/25] log/backend: Correct order of const in copy constructor Follows our predominant coding style. Also explicitly specifies the move constructor/assignment operator as well. --- src/common/logging/backend.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index cf955b0930..9f99b6a5f9 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -38,8 +38,11 @@ public: return backend; } - Impl(Impl const&) = delete; - const Impl& operator=(Impl const&) = delete; + Impl(const Impl&) = delete; + Impl& operator=(const Impl&) = delete; + + Impl(Impl&&) = delete; + Impl& operator=(Impl&&) = delete; void PushEntry(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, std::string message) { From 399f3d4e32cb329a23d8c97cd9152aa908ff4a77 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 20 Apr 2021 12:53:02 -0400 Subject: [PATCH 04/25] log/backend: Make use of erase_if Same behavior, but less verbose. --- src/common/logging/backend.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 9f99b6a5f9..f47e2368f4 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -57,10 +57,10 @@ public: void RemoveBackend(std::string_view backend_name) { std::lock_guard lock{writing_mutex}; - const auto it = - std::remove_if(backends.begin(), backends.end(), - [&backend_name](const auto& i) { return backend_name == i->GetName(); }); - backends.erase(it, backends.end()); + + std::erase_if(backends, [&backend_name](const auto& backend) { + return backend_name == backend->GetName(); + }); } Backend* GetBackend(std::string_view backend_name) { From 5c86147ef4e2e0f5d622941c34764f2b5a9a439c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 20 Apr 2021 12:57:45 -0400 Subject: [PATCH 05/25] log/backend: Use in-class initializer for FileBackend We can also avoid redundant constructions of the same string repeatedly. --- src/common/logging/backend.cpp | 10 ++++++---- src/common/logging/backend.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index f47e2368f4..cdfa0964a6 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -145,12 +145,14 @@ void LogcatBackend::Write(const Entry& entry) { PrintMessageToLogcat(entry); } -FileBackend::FileBackend(const std::string& filename) : bytes_written(0) { - if (FileUtil::Exists(filename + ".old.txt")) { - FileUtil::Delete(filename + ".old.txt"); +FileBackend::FileBackend(const std::string& filename) { + const auto old_filename = filename + ".old.txt"; + + if (FileUtil::Exists(old_filename)) { + FileUtil::Delete(old_filename); } if (FileUtil::Exists(filename)) { - FileUtil::Rename(filename, filename + ".old.txt"); + FileUtil::Rename(filename, old_filename); } // _SH_DENYWR allows read only access to the file for other programs. diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index eeb360aa49..b9e5386a07 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -109,7 +109,7 @@ public: private: FileUtil::IOFile file; - std::size_t bytes_written; + std::size_t bytes_written = 0; }; /** From b559c078bc80859f9f5d54c668415535010076b8 Mon Sep 17 00:00:00 2001 From: Morph <39850852+Morph1984@users.noreply.github.com> Date: Sat, 24 Jun 2023 02:00:26 +0300 Subject: [PATCH 06/25] common: logging: backend: Wrap IOFile in a unique_ptr Allows us to forward declare FileUtil::IOFile. --- src/common/logging/backend.cpp | 17 +++++++++++++---- src/common/logging/backend.h | 18 ++++++++++++++++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index cdfa0964a6..1fcd482f7c 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -16,6 +16,7 @@ #define _SH_DENYWR 0 #endif #include "common/assert.h" +#include "common/file_util.h" #include "common/logging/backend.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" @@ -133,14 +134,20 @@ private: std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; }; +ConsoleBackend::~ConsoleBackend() = default; + void ConsoleBackend::Write(const Entry& entry) { PrintMessage(entry); } +ColorConsoleBackend::~ColorConsoleBackend() = default; + void ColorConsoleBackend::Write(const Entry& entry) { PrintColoredMessage(entry); } +LogcatBackend::~LogcatBackend() = default; + void LogcatBackend::Write(const Entry& entry) { PrintMessageToLogcat(entry); } @@ -157,19 +164,21 @@ FileBackend::FileBackend(const std::string& filename) { // _SH_DENYWR allows read only access to the file for other programs. // It is #defined to 0 on other platforms - file = FileUtil::IOFile(filename, "w", _SH_DENYWR); + file = std::make_unique(filename, "w", _SH_DENYWR); } +FileBackend::~FileBackend() = default; + void FileBackend::Write(const Entry& entry) { // prevent logs from going over the maximum size (in case its spamming and the user doesn't // know) constexpr std::size_t MAX_BYTES_WRITTEN = 50 * 1024L * 1024L; - if (!file.IsOpen() || bytes_written > MAX_BYTES_WRITTEN) { + if (!file->IsOpen() || bytes_written > MAX_BYTES_WRITTEN) { return; } - bytes_written += file.WriteString(FormatLogMessage(entry).append(1, '\n')); + bytes_written += file->WriteString(FormatLogMessage(entry).append(1, '\n')); if (entry.log_level >= Level::Error) { - file.Flush(); + file->Flush(); } } diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index b9e5386a07..47b084d340 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -8,10 +8,13 @@ #include #include #include -#include "common/file_util.h" #include "common/logging/filter.h" #include "common/logging/log.h" +namespace FileUtil { +class IOFile; +} + namespace Common::Log { /** @@ -36,6 +39,7 @@ struct Entry { class Backend { public: virtual ~Backend() = default; + virtual void SetFilter(const Filter& new_filter) { filter = new_filter; } @@ -51,6 +55,8 @@ private: */ class ConsoleBackend : public Backend { public: + ~ConsoleBackend() override; + static const char* Name() { return "console"; } @@ -65,6 +71,8 @@ public: */ class ColorConsoleBackend : public Backend { public: + ~ColorConsoleBackend() override; + static const char* Name() { return "color_console"; } @@ -80,6 +88,8 @@ public: */ class LogcatBackend : public Backend { public: + ~LogcatBackend() override; + static const char* Name() { return "logcat"; } @@ -95,6 +105,8 @@ public: */ class FileBackend : public Backend { public: + ~FileBackend() override; + explicit FileBackend(const std::string& filename); static const char* Name() { @@ -108,7 +120,7 @@ public: void Write(const Entry& entry) override; private: - FileUtil::IOFile file; + std::unique_ptr file; std::size_t bytes_written = 0; }; @@ -117,6 +129,8 @@ private: */ class DebuggerBackend : public Backend { public: + ~DebuggerBackend() override; + static const char* Name() { return "debugger"; } From b57773b1cfd6b342d3a1867d31f223d210a47f7c Mon Sep 17 00:00:00 2001 From: Morph <39850852+Morph1984@users.noreply.github.com> Date: Sat, 24 Jun 2023 02:27:24 +0300 Subject: [PATCH 07/25] common: logging: Restructure backend code --- .../configuration/configure_debug.cpp | 2 +- src/common/CMakeLists.txt | 1 + src/common/logging/backend.cpp | 128 ++-------------- src/common/logging/backend.h | 29 +--- src/common/logging/filter.cpp | 112 +++++++++++++- src/common/logging/filter.h | 55 ++++++- src/common/logging/log.h | 143 +----------------- src/common/logging/text_formatter.cpp | 2 +- src/common/logging/types.h | 123 +++++++++++++++ 9 files changed, 310 insertions(+), 285 deletions(-) create mode 100644 src/common/logging/types.h diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index 5ec1414672..d0b2441339 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -9,7 +9,7 @@ #include "citra_qt/debugger/console.h" #include "citra_qt/uisettings.h" #include "common/file_util.h" -#include "common/logging/log.h" +#include "common/logging/backend.h" #include "common/settings.h" #include "core/core.h" #include "ui_configure_debug.h" diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 14a21b867c..9063329513 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -86,6 +86,7 @@ add_library(citra_common STATIC logging/log.h logging/text_formatter.cpp logging/text_formatter.h + logging/types.h math_util.h memory_detect.cpp memory_detect.h diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 1fcd482f7c..4afba68330 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -15,7 +15,6 @@ #else #define _SH_DENYWR 0 #endif -#include "common/assert.h" #include "common/file_util.h" #include "common/logging/backend.h" #include "common/logging/log.h" @@ -25,10 +24,6 @@ namespace Common::Log { -Filter filter; -void SetGlobalFilter(const Filter& f) { - filter = f; -} /** * Static state as a singleton. */ @@ -64,6 +59,14 @@ public: }); } + const Filter& GetGlobalFilter() const { + return filter; + } + + void SetGlobalFilter(const Filter& f) { + filter = f; + } + Backend* GetBackend(std::string_view backend_name) { const auto it = std::find_if(backends.begin(), backends.end(), @@ -182,119 +185,16 @@ void FileBackend::Write(const Entry& entry) { } } +DebuggerBackend::~DebuggerBackend() = default; + void DebuggerBackend::Write(const Entry& entry) { #ifdef _WIN32 ::OutputDebugStringW(Common::UTF8ToUTF16W(FormatLogMessage(entry).append(1, '\n')).c_str()); #endif } -/// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. -#define ALL_LOG_CLASSES() \ - CLS(Log) \ - CLS(Common) \ - SUB(Common, Filesystem) \ - SUB(Common, Memory) \ - CLS(Core) \ - SUB(Core, ARM11) \ - SUB(Core, Timing) \ - SUB(Core, Cheats) \ - CLS(Config) \ - CLS(Debug) \ - SUB(Debug, Emulated) \ - SUB(Debug, GPU) \ - SUB(Debug, Breakpoint) \ - SUB(Debug, GDBStub) \ - CLS(Kernel) \ - SUB(Kernel, SVC) \ - CLS(Applet) \ - SUB(Applet, SWKBD) \ - CLS(Service) \ - SUB(Service, SRV) \ - SUB(Service, FRD) \ - SUB(Service, FS) \ - SUB(Service, ERR) \ - SUB(Service, APT) \ - SUB(Service, BOSS) \ - SUB(Service, GSP) \ - SUB(Service, AC) \ - SUB(Service, AM) \ - SUB(Service, PTM) \ - SUB(Service, LDR) \ - SUB(Service, MIC) \ - SUB(Service, NDM) \ - SUB(Service, NFC) \ - SUB(Service, NIM) \ - SUB(Service, NS) \ - SUB(Service, NWM) \ - SUB(Service, CAM) \ - SUB(Service, CECD) \ - SUB(Service, CFG) \ - SUB(Service, CSND) \ - SUB(Service, DSP) \ - SUB(Service, DLP) \ - SUB(Service, HID) \ - SUB(Service, HTTP) \ - SUB(Service, SOC) \ - SUB(Service, IR) \ - SUB(Service, Y2R) \ - SUB(Service, PS) \ - SUB(Service, PLGLDR) \ - CLS(HW) \ - SUB(HW, Memory) \ - SUB(HW, LCD) \ - SUB(HW, GPU) \ - SUB(HW, AES) \ - CLS(Frontend) \ - CLS(Render) \ - SUB(Render, Software) \ - SUB(Render, OpenGL) \ - SUB(Render, Vulkan) \ - CLS(Audio) \ - SUB(Audio, DSP) \ - SUB(Audio, Sink) \ - CLS(Input) \ - CLS(Network) \ - CLS(Movie) \ - CLS(Loader) \ - CLS(WebService) \ - CLS(RPC_Server) - -// GetClassName is a macro defined by Windows.h, grrr... -const char* GetLogClassName(Class log_class) { - switch (log_class) { -#define CLS(x) \ - case Class::x: \ - return #x; -#define SUB(x, y) \ - case Class::x##_##y: \ - return #x "." #y; - ALL_LOG_CLASSES() -#undef CLS -#undef SUB - case Class::Count: - default: - break; - } - UNREACHABLE(); -} - -const char* GetLevelName(Level log_level) { -#define LVL(x) \ - case Level::x: \ - return #x - switch (log_level) { - LVL(Trace); - LVL(Debug); - LVL(Info); - LVL(Warning); - LVL(Error); - LVL(Critical); - case Level::Count: - default: - break; - } -#undef LVL - UNREACHABLE(); +void SetGlobalFilter(const Filter& filter) { + Impl::Instance().SetGlobalFilter(filter); } void AddBackend(std::unique_ptr backend) { @@ -313,6 +213,10 @@ void FmtLogMessageImpl(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, const fmt::format_args& args) { auto& instance = Impl::Instance(); + const auto& filter = instance.GetGlobalFilter(); + if (!filter.CheckMessage(log_class, log_level)) + return; + instance.PushEntry(log_class, log_level, filename, line_num, function, fmt::vformat(format, args)); } diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 47b084d340..4d91179513 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include #include @@ -17,21 +16,6 @@ class IOFile; namespace Common::Log { -/** - * A log entry. Log entries are store in a structured format to permit more varied output - * formatting on different frontends, as well as facilitating filtering and aggregation. - */ -struct Entry { - std::chrono::microseconds timestamp; - Class log_class{}; - Level log_level{}; - const char* filename = nullptr; - unsigned int line_num = 0; - std::string function; - std::string message; - bool final_entry = false; -}; - /** * Interface for logging backends. As loggers can be created and removed at runtime, this can be * used by a frontend for adding a custom logging backend as needed @@ -147,14 +131,9 @@ void RemoveBackend(std::string_view backend_name); Backend* GetBackend(std::string_view backend_name); /** - * Returns the name of the passed log class as a C-string. Subclasses are separated by periods - * instead of underscores as in the enumeration. + * The global filter will prevent any messages from even being processed if they are filtered. Each + * backend can have a filter, but if the level is lower than the global filter, the backend will + * never get the message */ -const char* GetLogClassName(Class log_class); - -/** - * Returns the name of the passed log level as a C-string. - */ -const char* GetLevelName(Level log_level); - +void SetGlobalFilter(const Filter& filter); } // namespace Common::Log diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index 942873575f..0d33872c84 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -3,7 +3,6 @@ // Refer to the license.txt file included. #include -#include "common/logging/backend.h" #include "common/logging/filter.h" #include "common/string_util.h" @@ -22,7 +21,7 @@ Level GetLevelByName(const It begin, const It end) { template Class GetClassByName(const It begin, const It end) { - for (ClassType i = 0; i < static_cast(Class::Count); ++i) { + for (u8 i = 0; i < static_cast(Class::Count); ++i) { const char* level_name = GetLogClassName(static_cast(i)); if (Common::ComparePartialString(begin, end, level_name)) { return static_cast(i); @@ -62,6 +61,115 @@ bool ParseFilterRule(Filter& instance, Iterator begin, Iterator end) { } } // Anonymous namespace +/// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. +#define ALL_LOG_CLASSES() \ + CLS(Log) \ + CLS(Common) \ + SUB(Common, Filesystem) \ + SUB(Common, Memory) \ + CLS(Core) \ + SUB(Core, ARM11) \ + SUB(Core, Timing) \ + SUB(Core, Cheats) \ + CLS(Config) \ + CLS(Debug) \ + SUB(Debug, Emulated) \ + SUB(Debug, GPU) \ + SUB(Debug, Breakpoint) \ + SUB(Debug, GDBStub) \ + CLS(Kernel) \ + SUB(Kernel, SVC) \ + CLS(Applet) \ + SUB(Applet, SWKBD) \ + CLS(Service) \ + SUB(Service, SRV) \ + SUB(Service, FRD) \ + SUB(Service, FS) \ + SUB(Service, ERR) \ + SUB(Service, APT) \ + SUB(Service, BOSS) \ + SUB(Service, GSP) \ + SUB(Service, AC) \ + SUB(Service, AM) \ + SUB(Service, PTM) \ + SUB(Service, LDR) \ + SUB(Service, MIC) \ + SUB(Service, NDM) \ + SUB(Service, NFC) \ + SUB(Service, NIM) \ + SUB(Service, NS) \ + SUB(Service, NWM) \ + SUB(Service, CAM) \ + SUB(Service, CECD) \ + SUB(Service, CFG) \ + SUB(Service, CSND) \ + SUB(Service, DSP) \ + SUB(Service, DLP) \ + SUB(Service, HID) \ + SUB(Service, HTTP) \ + SUB(Service, SOC) \ + SUB(Service, IR) \ + SUB(Service, Y2R) \ + SUB(Service, PS) \ + SUB(Service, PLGLDR) \ + CLS(HW) \ + SUB(HW, Memory) \ + SUB(HW, LCD) \ + SUB(HW, GPU) \ + SUB(HW, AES) \ + CLS(Frontend) \ + CLS(Render) \ + SUB(Render, Software) \ + SUB(Render, OpenGL) \ + SUB(Render, Vulkan) \ + CLS(Audio) \ + SUB(Audio, DSP) \ + SUB(Audio, Sink) \ + CLS(Input) \ + CLS(Network) \ + CLS(Movie) \ + CLS(Loader) \ + CLS(WebService) \ + CLS(RPC_Server) + +// GetClassName is a macro defined by Windows.h, grrr... +const char* GetLogClassName(Class log_class) { + switch (log_class) { +#define CLS(x) \ + case Class::x: \ + return #x; +#define SUB(x, y) \ + case Class::x##_##y: \ + return #x "." #y; + ALL_LOG_CLASSES() +#undef CLS +#undef SUB + case Class::Count: + default: + break; + } + UNREACHABLE(); +} + +const char* GetLevelName(Level log_level) { +#define LVL(x) \ + case Level::x: \ + return #x + switch (log_level) { + LVL(Trace); + LVL(Debug); + LVL(Info); + LVL(Warning); + LVL(Error); + LVL(Critical); + case Level::Count: + default: + break; + } +#undef LVL + UNREACHABLE(); +} + Filter::Filter(Level default_level) { ResetAll(default_level); } diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index 3a8f23e368..26d5f73929 100644 --- a/src/common/logging/filter.h +++ b/src/common/logging/filter.h @@ -7,6 +7,57 @@ #include #include #include -#include "common/logging/log.h" +#include "common/logging/types.h" -namespace Common::Log {} // namespace Common::Log +namespace Common::Log { + +/** + * Returns the name of the passed log class as a C-string. Subclasses are separated by periods + * instead of underscores as in the enumeration. + */ +const char* GetLogClassName(Class log_class); + +/** + * Returns the name of the passed log level as a C-string. + */ +const char* GetLevelName(Level log_level); + +/** + * Implements a log message filter which allows different log classes to have different minimum + * severity levels. The filter can be changed at runtime and can be parsed from a string to allow + * editing via the interface or loading from a configuration file. + */ +class Filter { +public: + /// Initializes the filter with all classes having `default_level` as the minimum level. + explicit Filter(Level default_level = Level::Info); + + /// Resets the filter so that all classes have `level` as the minimum displayed level. + void ResetAll(Level level); + /// Sets the minimum level of `log_class` (and not of its subclasses) to `level`. + void SetClassLevel(Class log_class, Level level); + + /** + * Parses a filter string and applies it to this filter. + * + * A filter string consists of a space-separated list of filter rules, each of the format + * `:`. `` is a log class name, with subclasses separated using periods. + * `*` is allowed as a class name and will reset all filters to the specified level. `` + * a severity level name which will be set as the minimum logging level of the matched classes. + * Rules are applied left to right, with each rule overriding previous ones in the sequence. + * + * A few examples of filter rules: + * - `*:Info` -- Resets the level of all classes to Info. + * - `Service:Info` -- Sets the level of Service to Info. + * - `Service.FS:Trace` -- Sets the level of the Service.FS class to Trace. + */ + void ParseFilterString(std::string_view filter_view); + + /// Matches class/level combination against the filter, returning true if it passed. + bool CheckMessage(Class log_class, Level level) const; + +private: + std::array(Class::Count)> class_levels; +}; + +} // namespace Common::Log diff --git a/src/common/logging/log.h b/src/common/logging/log.h index e6a3ea5b22..ab596d08f7 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -6,8 +6,8 @@ #include #include -#include "common/common_types.h" #include "common/logging/formatter.h" +#include "common/logging/types.h" namespace Common::Log { @@ -20,144 +20,6 @@ constexpr const char* TrimSourcePath(std::string_view source) { return source.data() + idx; } -/// Specifies the severity or level of detail of the log message. -enum class Level : u8 { - Trace, ///< Extremely detailed and repetitive debugging information that is likely to - ///< pollute logs. - Debug, ///< Less detailed debugging information. - Info, ///< Status information from important points during execution. - Warning, ///< Minor or potential problems found during execution of a task. - Error, ///< Major problems found during execution of a task that prevent it from being - ///< completed. - Critical, ///< Major problems during execution that threaten the stability of the entire - ///< application. - - Count ///< Total number of logging levels -}; - -typedef u8 ClassType; - -/** - * Specifies the sub-system that generated the log message. - * - * @note If you add a new entry here, also add a corresponding one to `ALL_LOG_CLASSES` in - * backend.cpp. - */ -enum class Class : ClassType { - Log, ///< Messages about the log system itself - Common, ///< Library routines - Common_Filesystem, ///< Filesystem interface library - Common_Memory, ///< Memory mapping and management functions - Core, ///< LLE emulation core - Core_ARM11, ///< ARM11 CPU core - Core_Timing, ///< CoreTiming functions - Core_Cheats, ///< Cheat functions - Config, ///< Emulator configuration (including commandline) - Debug, ///< Debugging tools - Debug_Emulated, ///< Debug messages from the emulated programs - Debug_GPU, ///< GPU debugging tools - Debug_Breakpoint, ///< Logging breakpoints and watchpoints - Debug_GDBStub, ///< GDB Stub - Kernel, ///< The HLE implementation of the CTR kernel - Kernel_SVC, ///< Kernel system calls - Applet, ///< HLE implementation of system applets. Each applet - ///< should have its own subclass. - Applet_SWKBD, ///< The Software Keyboard applet - Service, ///< HLE implementation of system services. Each major service - ///< should have its own subclass. - Service_SRV, ///< The SRV (Service Directory) implementation - Service_FRD, ///< The FRD (Friends) service - Service_FS, ///< The FS (Filesystem) service implementation - Service_ERR, ///< The ERR (Error) port implementation - Service_APT, ///< The APT (Applets) service - Service_BOSS, ///< The BOSS (SpotPass) service - Service_GSP, ///< The GSP (GPU control) service - Service_AC, ///< The AC (WiFi status) service - Service_AM, ///< The AM (Application manager) service - Service_PTM, ///< The PTM (Power status & misc.) service - Service_LDR, ///< The LDR (3ds dll loader) service - Service_MIC, ///< The MIC (Microphone) service - Service_NDM, ///< The NDM (Network daemon manager) service - Service_NFC, ///< The NFC service - Service_NIM, ///< The NIM (Network interface manager) service - Service_NS, ///< The NS (Nintendo User Interface Shell) service - Service_NWM, ///< The NWM (Network wlan manager) service - Service_CAM, ///< The CAM (Camera) service - Service_CECD, ///< The CECD (StreetPass) service - Service_CFG, ///< The CFG (Configuration) service - Service_CSND, ///< The CSND (CWAV format process) service - Service_DSP, ///< The DSP (DSP control) service - Service_DLP, ///< The DLP (Download Play) service - Service_HID, ///< The HID (Human interface device) service - Service_HTTP, ///< The HTTP service - Service_SOC, ///< The SOC (Socket) service - Service_IR, ///< The IR service - Service_Y2R, ///< The Y2R (YUV to RGB conversion) service - Service_PS, ///< The PS (Process) service - Service_PLGLDR, ///< The PLGLDR (plugin loader) service - HW, ///< Low-level hardware emulation - HW_Memory, ///< Memory-map and address translation - HW_LCD, ///< LCD register emulation - HW_GPU, ///< GPU control emulation - HW_AES, ///< AES engine emulation - Frontend, ///< Emulator UI - Render, ///< Emulator video output and hardware acceleration - Render_Software, ///< Software renderer backend - Render_OpenGL, ///< OpenGL backend - Render_Vulkan, ///< Vulkan backend - Audio, ///< Audio emulation - Audio_DSP, ///< The HLE and LLE implementations of the DSP - Audio_Sink, ///< Emulator audio output backend - Loader, ///< ROM loader - Input, ///< Input emulation - Network, ///< Network emulation - Movie, ///< Movie (Input Recording) Playback - WebService, ///< Interface to Citra Web Services - RPC_Server, ///< RPC server - Count ///< Total number of logging classes -}; - -/** - * Implements a log message filter which allows different log classes to have different minimum - * severity levels. The filter can be changed at runtime and can be parsed from a string to allow - * editing via the interface or loading from a configuration file. - */ -class Filter { -public: - /// Initializes the filter with all classes having `default_level` as the minimum level. - explicit Filter(Level default_level = Level::Info); - - /// Resets the filter so that all classes have `level` as the minimum displayed level. - void ResetAll(Level level); - /// Sets the minimum level of `log_class` (and not of its subclasses) to `level`. - void SetClassLevel(Class log_class, Level level); - - /** - * Parses a filter string and applies it to this filter. - * - * A filter string consists of a space-separated list of filter rules, each of the format - * `:`. `` is a log class name, with subclasses separated using periods. - * `*` is allowed as a class name and will reset all filters to the specified level. `` - * a severity level name which will be set as the minimum logging level of the matched classes. - * Rules are applied left to right, with each rule overriding previous ones in the sequence. - * - * A few examples of filter rules: - * - `*:Info` -- Resets the level of all classes to Info. - * - `Service:Info` -- Sets the level of Service to Info. - * - `Service.FS:Trace` -- Sets the level of the Service.FS class to Trace. - */ - void ParseFilterString(std::string_view filter_view); - - /// Matches class/level combination against the filter, returning true if it passed. - bool CheckMessage(Class log_class, Level level) const; - -private: - std::array(Class::Count)> class_levels; -}; -extern Filter filter; - -void SetGlobalFilter(const Filter& f); - /// Logs a message to the global logger, using fmt void FmtLogMessageImpl(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, @@ -166,9 +28,6 @@ void FmtLogMessageImpl(Class log_class, Level log_level, const char* filename, template void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, const Args&... args) { - if (!filter.CheckMessage(log_class, log_level)) - return; - FmtLogMessageImpl(log_class, log_level, filename, line_num, function, format, fmt::make_format_args(args...)); } diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index e66bfa43f8..9942dd76b3 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -13,7 +13,7 @@ #include "common/assert.h" #include "common/common_funcs.h" -#include "common/logging/backend.h" +#include "common/logging/filter.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" #include "common/string_util.h" diff --git a/src/common/logging/types.h b/src/common/logging/types.h new file mode 100644 index 0000000000..8dd0b74441 --- /dev/null +++ b/src/common/logging/types.h @@ -0,0 +1,123 @@ +// Copyright 2023 Citra Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +#include + +#include "common/common_types.h" + +namespace Common::Log { + +/// Specifies the severity or level of detail of the log message. +enum class Level : u8 { + Trace, ///< Extremely detailed and repetitive debugging information that is likely to + ///< pollute logs. + Debug, ///< Less detailed debugging information. + Info, ///< Status information from important points during execution. + Warning, ///< Minor or potential problems found during execution of a task. + Error, ///< Major problems found during execution of a task that prevent it from being + ///< completed. + Critical, ///< Major problems during execution that threaten the stability of the entire + ///< application. + + Count ///< Total number of logging levels +}; + +/** + * Specifies the sub-system that generated the log message. + * + * @note If you add a new entry here, also add a corresponding one to `ALL_LOG_CLASSES` in + * backend.cpp. + */ +enum class Class : u8 { + Log, ///< Messages about the log system itself + Common, ///< Library routines + Common_Filesystem, ///< Filesystem interface library + Common_Memory, ///< Memory mapping and management functions + Core, ///< LLE emulation core + Core_ARM11, ///< ARM11 CPU core + Core_Timing, ///< CoreTiming functions + Core_Cheats, ///< Cheat functions + Config, ///< Emulator configuration (including commandline) + Debug, ///< Debugging tools + Debug_Emulated, ///< Debug messages from the emulated programs + Debug_GPU, ///< GPU debugging tools + Debug_Breakpoint, ///< Logging breakpoints and watchpoints + Debug_GDBStub, ///< GDB Stub + Kernel, ///< The HLE implementation of the CTR kernel + Kernel_SVC, ///< Kernel system calls + Applet, ///< HLE implementation of system applets. Each applet + ///< should have its own subclass. + Applet_SWKBD, ///< The Software Keyboard applet + Service, ///< HLE implementation of system services. Each major service + ///< should have its own subclass. + Service_SRV, ///< The SRV (Service Directory) implementation + Service_FRD, ///< The FRD (Friends) service + Service_FS, ///< The FS (Filesystem) service implementation + Service_ERR, ///< The ERR (Error) port implementation + Service_APT, ///< The APT (Applets) service + Service_BOSS, ///< The BOSS (SpotPass) service + Service_GSP, ///< The GSP (GPU control) service + Service_AC, ///< The AC (WiFi status) service + Service_AM, ///< The AM (Application manager) service + Service_PTM, ///< The PTM (Power status & misc.) service + Service_LDR, ///< The LDR (3ds dll loader) service + Service_MIC, ///< The MIC (Microphone) service + Service_NDM, ///< The NDM (Network daemon manager) service + Service_NFC, ///< The NFC service + Service_NIM, ///< The NIM (Network interface manager) service + Service_NS, ///< The NS (Nintendo User Interface Shell) service + Service_NWM, ///< The NWM (Network wlan manager) service + Service_CAM, ///< The CAM (Camera) service + Service_CECD, ///< The CECD (StreetPass) service + Service_CFG, ///< The CFG (Configuration) service + Service_CSND, ///< The CSND (CWAV format process) service + Service_DSP, ///< The DSP (DSP control) service + Service_DLP, ///< The DLP (Download Play) service + Service_HID, ///< The HID (Human interface device) service + Service_HTTP, ///< The HTTP service + Service_SOC, ///< The SOC (Socket) service + Service_IR, ///< The IR service + Service_Y2R, ///< The Y2R (YUV to RGB conversion) service + Service_PS, ///< The PS (Process) service + Service_PLGLDR, ///< The PLGLDR (plugin loader) service + HW, ///< Low-level hardware emulation + HW_Memory, ///< Memory-map and address translation + HW_LCD, ///< LCD register emulation + HW_GPU, ///< GPU control emulation + HW_AES, ///< AES engine emulation + Frontend, ///< Emulator UI + Render, ///< Emulator video output and hardware acceleration + Render_Software, ///< Software renderer backend + Render_OpenGL, ///< OpenGL backend + Render_Vulkan, ///< Vulkan backend + Audio, ///< Audio emulation + Audio_DSP, ///< The HLE and LLE implementations of the DSP + Audio_Sink, ///< Emulator audio output backend + Loader, ///< ROM loader + Input, ///< Input emulation + Network, ///< Network emulation + Movie, ///< Movie (Input Recording) Playback + WebService, ///< Interface to Citra Web Services + RPC_Server, ///< RPC server + Count ///< Total number of logging classes +}; + +/** + * A log entry. Log entries are store in a structured format to permit more varied output + * formatting on different frontends, as well as facilitating filtering and aggregation. + */ +struct Entry { + std::chrono::microseconds timestamp; + Class log_class{}; + Level log_level{}; + const char* filename = nullptr; + unsigned int line_num = 0; + std::string function; + std::string message; + bool final_entry = false; +}; + +} // namespace Common::Log From 8e8ca7d9d02388eefa1b1afa73b935349a6165a0 Mon Sep 17 00:00:00 2001 From: Morph <39850852+Morph1984@users.noreply.github.com> Date: Sat, 24 Jun 2023 02:30:06 +0300 Subject: [PATCH 08/25] common: logging: backend: Close the file after exceeding the write limit There's no point in keeping the file open after the write limit is exceeded. This allows the file to be committed to the disk shortly after it is closed and avoids redundantly checking whether or not the write limit is exceeded. --- src/common/logging/backend.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 4afba68330..3c8f8f54d5 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -173,12 +173,19 @@ FileBackend::FileBackend(const std::string& filename) { FileBackend::~FileBackend() = default; void FileBackend::Write(const Entry& entry) { - // prevent logs from going over the maximum size (in case its spamming and the user doesn't - // know) - constexpr std::size_t MAX_BYTES_WRITTEN = 50 * 1024L * 1024L; - if (!file->IsOpen() || bytes_written > MAX_BYTES_WRITTEN) { + if (!file->IsOpen()) { return; } + + // Prevent logs from exceeding a set maximum size in the event that log entries are spammed. + constexpr std::size_t MAX_BYTES_WRITTEN = 50 * 1024L * 1024L; + + // Close the file after the write limit is exceeded. + if (bytes_written > MAX_BYTES_WRITTEN) { + file->Close(); + return; + } + bytes_written += file->WriteString(FormatLogMessage(entry).append(1, '\n')); if (entry.log_level >= Level::Error) { file->Flush(); From 3641b9891d7348e279f4e8853a9dadbfd491adfb Mon Sep 17 00:00:00 2001 From: yzct12345 <87620833+yzct12345@users.noreply.github.com> Date: Sat, 24 Jun 2023 02:58:49 +0300 Subject: [PATCH 09/25] logging: Simplify and make thread-safe This simplifies the logging system. This also fixes some lost messages on startup. The simplification is simple. I removed unused functions and moved most things in the .h to the .cpp. I replaced the unnecessary linked list with its contents laid out as three member variables. Anything that went through the linked list now directly accesses the backends. Generic functions are replaced with those for each specific use case and there aren't many. This change increases coupling but we gain back more KISS and encapsulation. With those changes it was easy to make it thread-safe. I just removed the mutex and turned a boolean atomic. I was planning to use this thread-safety in my next PR about stacktraces. It was actually async-signal-safety at first but I ended up using a different approach. Anyway getting rid of the linked list is important for that because have the list of backends constantly changing complicates things. --- src/citra/citra.cpp | 22 +- src/citra_qt/debugger/console.cpp | 11 +- src/citra_qt/main.cpp | 20 +- src/common/logging/backend.cpp | 381 ++++++++++++++++++----------- src/common/logging/backend.h | 130 +--------- src/common/logging/filter.cpp | 7 + src/common/logging/filter.h | 3 + src/core/core.cpp | 9 +- src/core/core.h | 9 +- src/dedicated_room/citra-room.cpp | 13 +- src/tests/common/param_package.cpp | 2 + 11 files changed, 282 insertions(+), 325 deletions(-) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index 7fc8188c8f..aade3ee9c9 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -174,25 +174,10 @@ static void OnStatusMessageReceived(const Network::StatusMessageEntry& msg) { std::cout << std::endl << "* " << message << std::endl << std::endl; } -static void InitializeLogging() { - using namespace Common; - - Log::Filter log_filter(Log::Level::Debug); - log_filter.ParseFilterString(Settings::values.log_filter.GetValue()); - Log::SetGlobalFilter(log_filter); - - Log::AddBackend(std::make_unique()); - - const std::string& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); - FileUtil::CreateFullPath(log_dir); - Log::AddBackend(std::make_unique(log_dir + LOG_FILE)); -#ifdef _WIN32 - Log::AddBackend(std::make_unique()); -#endif -} - /// Application entry point int main(int argc, char** argv) { + Common::Log::Initialize(); + Common::Log::SetColorConsoleBackendEnabled(true); Common::DetachedTasks detached_tasks; Config config; int option_index = 0; @@ -203,8 +188,6 @@ int main(int argc, char** argv) { std::string movie_play; std::string dump_video; - InitializeLogging(); - char* endarg; #ifdef _WIN32 int argc_w; @@ -346,6 +329,7 @@ int main(int argc, char** argv) { return -1; } + Core::System::InitializeGlobalInstance(); auto& system = Core::System::GetInstance(); auto& movie = Core::Movie::GetInstance(); diff --git a/src/citra_qt/debugger/console.cpp b/src/citra_qt/debugger/console.cpp index 4fb8d83dbe..8187b0b1d6 100644 --- a/src/citra_qt/debugger/console.cpp +++ b/src/citra_qt/debugger/console.cpp @@ -21,6 +21,7 @@ void ToggleConsole() { console_shown = UISettings::values.show_console.GetValue(); } + using namespace Common::Log; #ifdef _WIN32 FILE* temp; if (UISettings::values.show_console) { @@ -29,24 +30,20 @@ void ToggleConsole() { freopen_s(&temp, "CONIN$", "r", stdin); freopen_s(&temp, "CONOUT$", "w", stdout); freopen_s(&temp, "CONOUT$", "w", stderr); - Common::Log::AddBackend(std::make_unique()); + SetColorConsoleBackendEnabled(true); } } else { if (FreeConsole()) { // In order to close the console, we have to also detach the streams on it. // Just redirect them to NUL if there is no console window - Common::Log::RemoveBackend(Common::Log::ColorConsoleBackend::Name()); + SetColorConsoleBackendEnabled(false); freopen_s(&temp, "NUL", "r", stdin); freopen_s(&temp, "NUL", "w", stdout); freopen_s(&temp, "NUL", "w", stderr); } } #else - if (UISettings::values.show_console) { - Log::AddBackend(std::make_unique()); - } else { - Log::RemoveBackend(Log::ColorConsoleBackend::Name()); - } + SetColorConsoleBackendEnabled(UISettings::values.show_console.GetValue()); #endif } } // namespace Debugger diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index b69bcf3d0e..71f2937851 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -146,21 +146,6 @@ void GMainWindow::ShowTelemetryCallout() { const int GMainWindow::max_recent_files_item; -static void InitializeLogging() { - using namespace Common; - - Log::Filter log_filter; - log_filter.ParseFilterString(Settings::values.log_filter.GetValue()); - Log::SetGlobalFilter(log_filter); - - const std::string& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); - FileUtil::CreateFullPath(log_dir); - Log::AddBackend(std::make_unique(log_dir + LOG_FILE)); -#ifdef _WIN32 - Log::AddBackend(std::make_unique()); -#endif -} - static QString PrettyProductName() { #ifdef _WIN32 // After Windows 10 Version 2004, Microsoft decided to switch to a different notation: 20H2 @@ -186,7 +171,6 @@ static QString PrettyProductName() { GMainWindow::GMainWindow(Core::System& system_) : ui{std::make_unique()}, system{system_}, movie{Core::Movie::GetInstance()}, config{std::make_unique()}, emu_thread{nullptr} { - InitializeLogging(); Debugger::ToggleConsole(); Settings::LogSettings(); @@ -2869,6 +2853,7 @@ static Qt::HighDpiScaleFactorRoundingPolicy GetHighDpiRoundingPolicy() { } int main(int argc, char* argv[]) { + Common::Log::Initialize(); Common::DetachedTasks detached_tasks; MicroProfileOnThreadCreate("Frontend"); SCOPE_EXIT({ MicroProfileShutdown(); }); @@ -2893,7 +2878,8 @@ int main(int argc, char* argv[]) { // generating shaders setlocale(LC_ALL, "C"); - Core::System& system = Core::System::GetInstance(); + Core::System::InitializeGlobalInstance(); + auto& system{Core::System::GetInstance()}; GMainWindow main_window(system); // Register frontend applets diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 3c8f8f54d5..60d87b1b8a 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -2,13 +2,10 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. -#include #include -#include -#include -#include #include #include +#include "common/common_paths.h" #ifdef _WIN32 #include // For _SH_DENYWR #include // For OutputDebugStringW @@ -16,6 +13,9 @@ #define _SH_DENYWR 0 #endif #include "common/file_util.h" +#include "common/literals.h" +#include "common/settings.h" +#include "common/thread.h" #include "common/logging/backend.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" @@ -24,14 +24,176 @@ namespace Common::Log { +namespace { + +/** + * Interface for logging backends. + */ +class Backend { +public: + virtual ~Backend() = default; + + virtual void Write(const Entry& entry) = 0; + + virtual void EnableForStacktrace() = 0; + + virtual void Flush() = 0; +}; + +/** + * Backend that writes to stderr and with color + */ +class ColorConsoleBackend final : public Backend { +public: + explicit ColorConsoleBackend() = default; + + ~ColorConsoleBackend() override = default; + + void Write(const Entry& entry) override { + if (enabled.load(std::memory_order_relaxed)) { + PrintColoredMessage(entry); + } + } + + void Flush() override { + // stderr shouldn't be buffered + } + + void EnableForStacktrace() override { + enabled = true; + } + + void SetEnabled(bool enabled_) { + enabled = enabled_; + } + +private: + std::atomic_bool enabled{false}; +}; + +/** + * Backend that writes to a file passed into the constructor + */ +class FileBackend final : public Backend { +public: + explicit FileBackend(const std::string& filename) { + auto old_filename = filename; + old_filename += ".old.txt"; + + // Existence checks are done within the functions themselves. + // We don't particularly care if these succeed or not. + static_cast(FileUtil::Delete(old_filename)); + static_cast(FileUtil::Rename(filename, old_filename)); + + // _SH_DENYWR allows read only access to the file for other programs. + // It is #defined to 0 on other platforms + file = std::make_unique(filename, "w", _SH_DENYWR); + } + + ~FileBackend() override = default; + + void Write(const Entry& entry) override { + if (!enabled) { + return; + } + + bytes_written += file->WriteString(FormatLogMessage(entry).append(1, '\n')); + + using namespace Common::Literals; + // Prevent logs from exceeding a set maximum size in the event that log entries are spammed. + const auto write_limit = 100_MiB; + const bool write_limit_exceeded = bytes_written > write_limit; + if (entry.log_level >= Level::Error || write_limit_exceeded) { + if (write_limit_exceeded) { + // Stop writing after the write limit is exceeded. + // Don't close the file so we can print a stacktrace if necessary + enabled = false; + } + file->Flush(); + } + } + + void Flush() override { + file->Flush(); + } + + void EnableForStacktrace() override { + enabled = true; + bytes_written = 0; + } + +private: + std::unique_ptr file; + bool enabled = true; + std::size_t bytes_written = 0; +}; + +/** + * Backend that writes to Visual Studio's output window + */ +class DebuggerBackend final : public Backend { +public: + explicit DebuggerBackend() = default; + + ~DebuggerBackend() override = default; + + void Write(const Entry& entry) override { +#ifdef _WIN32 + ::OutputDebugStringW(UTF8ToUTF16W(FormatLogMessage(entry).append(1, '\n')).c_str()); +#endif + } + + void Flush() override {} + + void EnableForStacktrace() override {} +}; + +#ifdef ANDROID +/** + * Backend that writes to the Android logcat + */ +class LogcatBackend : public Backend { +public: + explicit LogcatBackend() = default; + + ~LogcatBackend() override = default; + + void Write(const Entry& entry) override { + PrintMessageToLogcat(entry); + } + + void Flush() override {} + + void EnableForStacktrace() override {} +}; +#endif + +bool initialization_in_progress_suppress_logging = false; + /** * Static state as a singleton. */ class Impl { public: static Impl& Instance() { - static Impl backend; - return backend; + if (!instance) { + abort(); + } + return *instance; + } + + static void Initialize() { + if (instance) { + abort(); + } + initialization_in_progress_suppress_logging = true; + const auto& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); + void(FileUtil::CreateDir(log_dir)); + Filter filter; + filter.ParseFilterString(Settings::values.log_filter.GetValue()); + instance = std::unique_ptr(new Impl(log_dir + LOG_FILE, filter), + Deleter); + initialization_in_progress_suppress_logging = false; } Impl(const Impl&) = delete; @@ -40,74 +202,54 @@ public: Impl(Impl&&) = delete; Impl& operator=(Impl&&) = delete; - void PushEntry(Class log_class, Level log_level, const char* filename, unsigned int line_num, - const char* function, std::string message) { - message_queue.Push( - CreateEntry(log_class, log_level, filename, line_num, function, std::move(message))); - } - - void AddBackend(std::unique_ptr backend) { - std::lock_guard lock{writing_mutex}; - backends.push_back(std::move(backend)); - } - - void RemoveBackend(std::string_view backend_name) { - std::lock_guard lock{writing_mutex}; - - std::erase_if(backends, [&backend_name](const auto& backend) { - return backend_name == backend->GetName(); - }); - } - - const Filter& GetGlobalFilter() const { - return filter; - } - void SetGlobalFilter(const Filter& f) { filter = f; } - Backend* GetBackend(std::string_view backend_name) { - const auto it = - std::find_if(backends.begin(), backends.end(), - [&backend_name](const auto& i) { return backend_name == i->GetName(); }); - if (it == backends.end()) - return nullptr; - return it->get(); + void SetColorConsoleBackendEnabled(bool enabled) { + color_console_backend.SetEnabled(enabled); + } + + void PushEntry(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, std::string message) { + if (!filter.CheckMessage(log_class, log_level)) + return; + const Entry& entry = + CreateEntry(log_class, log_level, filename, line_num, function, std::move(message)); + message_queue.Push(entry); } private: - Impl() { - backend_thread = std::thread([&] { - Entry entry; - auto write_logs = [&](Entry& e) { - std::lock_guard lock{writing_mutex}; - for (const auto& backend : backends) { - backend->Write(e); - } - }; - while (true) { - entry = message_queue.PopWait(); - if (entry.final_entry) { - break; - } - write_logs(entry); - } - - // 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. - constexpr int MAX_LOGS_TO_WRITE = 100; - int logs_written = 0; - while (logs_written++ < MAX_LOGS_TO_WRITE && message_queue.Pop(entry)) { - write_logs(entry); - } - }); - } + 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(); + } + })} {} ~Impl() { - Entry entry; - entry.final_entry = true; - message_queue.Push(entry); + StopBackendThread(); + } + + void StopBackendThread() { + Entry stop_entry{}; + stop_entry.final_entry = true; + message_queue.Push(stop_entry); backend_thread.join(); } @@ -129,102 +271,51 @@ private: }; } - std::mutex writing_mutex; - std::thread backend_thread; - std::vector> backends; - Common::MPSCQueue message_queue; + void ForEachBackend(auto lambda) { + lambda(static_cast(debugger_backend)); + lambda(static_cast(color_console_backend)); + lambda(static_cast(file_backend)); + } + + static void Deleter(Impl* ptr) { + delete ptr; + } + + static inline std::unique_ptr instance{nullptr, Deleter}; + Filter filter; + DebuggerBackend debugger_backend{}; + ColorConsoleBackend color_console_backend{}; + FileBackend file_backend; + + std::thread backend_thread; + MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; }; +} // namespace -ConsoleBackend::~ConsoleBackend() = default; - -void ConsoleBackend::Write(const Entry& entry) { - PrintMessage(entry); +void Initialize() { + Impl::Initialize(); } -ColorConsoleBackend::~ColorConsoleBackend() = default; - -void ColorConsoleBackend::Write(const Entry& entry) { - PrintColoredMessage(entry); -} - -LogcatBackend::~LogcatBackend() = default; - -void LogcatBackend::Write(const Entry& entry) { - PrintMessageToLogcat(entry); -} - -FileBackend::FileBackend(const std::string& filename) { - const auto old_filename = filename + ".old.txt"; - - if (FileUtil::Exists(old_filename)) { - FileUtil::Delete(old_filename); - } - if (FileUtil::Exists(filename)) { - FileUtil::Rename(filename, old_filename); - } - - // _SH_DENYWR allows read only access to the file for other programs. - // It is #defined to 0 on other platforms - file = std::make_unique(filename, "w", _SH_DENYWR); -} - -FileBackend::~FileBackend() = default; - -void FileBackend::Write(const Entry& entry) { - if (!file->IsOpen()) { - return; - } - - // Prevent logs from exceeding a set maximum size in the event that log entries are spammed. - constexpr std::size_t MAX_BYTES_WRITTEN = 50 * 1024L * 1024L; - - // Close the file after the write limit is exceeded. - if (bytes_written > MAX_BYTES_WRITTEN) { - file->Close(); - return; - } - - bytes_written += file->WriteString(FormatLogMessage(entry).append(1, '\n')); - if (entry.log_level >= Level::Error) { - file->Flush(); - } -} - -DebuggerBackend::~DebuggerBackend() = default; - -void DebuggerBackend::Write(const Entry& entry) { -#ifdef _WIN32 - ::OutputDebugStringW(Common::UTF8ToUTF16W(FormatLogMessage(entry).append(1, '\n')).c_str()); -#endif +void DisableLoggingInTests() { + initialization_in_progress_suppress_logging = true; } void SetGlobalFilter(const Filter& filter) { Impl::Instance().SetGlobalFilter(filter); } -void AddBackend(std::unique_ptr backend) { - Impl::Instance().AddBackend(std::move(backend)); -} - -void RemoveBackend(std::string_view backend_name) { - Impl::Instance().RemoveBackend(backend_name); -} - -Backend* GetBackend(std::string_view backend_name) { - return Impl::Instance().GetBackend(backend_name); +void SetColorConsoleBackendEnabled(bool enabled) { + Impl::Instance().SetColorConsoleBackendEnabled(enabled); } void FmtLogMessageImpl(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, const fmt::format_args& args) { - auto& instance = Impl::Instance(); - const auto& filter = instance.GetGlobalFilter(); - if (!filter.CheckMessage(log_class, log_level)) - return; - - instance.PushEntry(log_class, log_level, filename, line_num, function, - fmt::vformat(format, args)); + if (!initialization_in_progress_suppress_logging) { + Impl::Instance().PushEntry(log_class, log_level, filename, line_num, function, + fmt::vformat(format, args)); + } } } // namespace Common::Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 4d91179513..cb7839ee93 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -4,136 +4,22 @@ #pragma once -#include -#include -#include +#include #include "common/logging/filter.h" -#include "common/logging/log.h" - -namespace FileUtil { -class IOFile; -} namespace Common::Log { -/** - * Interface for logging backends. As loggers can be created and removed at runtime, this can be - * used by a frontend for adding a custom logging backend as needed - */ -class Backend { -public: - virtual ~Backend() = default; +class Filter; - virtual void SetFilter(const Filter& new_filter) { - filter = new_filter; - } - virtual const char* GetName() const = 0; - virtual void Write(const Entry& entry) = 0; +/// Initializes the logging system. This should be the first thing called in main. +void Initialize(); -private: - Filter filter; -}; +void DisableLoggingInTests(); /** - * Backend that writes to stderr without any color commands - */ -class ConsoleBackend : public Backend { -public: - ~ConsoleBackend() override; - - static const char* Name() { - return "console"; - } - const char* GetName() const override { - return Name(); - } - void Write(const Entry& entry) override; -}; - -/** - * Backend that writes to stderr and with color - */ -class ColorConsoleBackend : public Backend { -public: - ~ColorConsoleBackend() override; - - static const char* Name() { - return "color_console"; - } - - const char* GetName() const override { - return Name(); - } - void Write(const Entry& entry) override; -}; - -/** - * Backend that writes to the Android logcat - */ -class LogcatBackend : public Backend { -public: - ~LogcatBackend() override; - - static const char* Name() { - return "logcat"; - } - - const char* GetName() const override { - return Name(); - } - void Write(const Entry& entry) override; -}; - -/** - * Backend that writes to a file passed into the constructor - */ -class FileBackend : public Backend { -public: - ~FileBackend() override; - - explicit FileBackend(const std::string& filename); - - static const char* Name() { - return "file"; - } - - const char* GetName() const override { - return Name(); - } - - void Write(const Entry& entry) override; - -private: - std::unique_ptr file; - std::size_t bytes_written = 0; -}; - -/** - * Backend that writes to Visual Studio's output window - */ -class DebuggerBackend : public Backend { -public: - ~DebuggerBackend() override; - - static const char* Name() { - return "debugger"; - } - const char* GetName() const override { - return Name(); - } - void Write(const Entry& entry) override; -}; - -void AddBackend(std::unique_ptr backend); - -void RemoveBackend(std::string_view backend_name); - -Backend* GetBackend(std::string_view backend_name); - -/** - * The global filter will prevent any messages from even being processed if they are filtered. Each - * backend can have a filter, but if the level is lower than the global filter, the backend will - * never get the message + * The global filter will prevent any messages from even being processed if they are filtered. */ void SetGlobalFilter(const Filter& filter); + +void SetColorConsoleBackendEnabled(bool enabled); } // namespace Common::Log diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index 0d33872c84..6213e3f49b 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -204,4 +204,11 @@ bool Filter::CheckMessage(Class log_class, Level level) const { return static_cast(level) >= static_cast(class_levels[static_cast(log_class)]); } + +bool Filter::IsDebug() const { + return std::any_of(class_levels.begin(), class_levels.end(), [](const Level& l) { + return static_cast(l) <= static_cast(Level::Debug); + }); +} + } // namespace Common::Log diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index 26d5f73929..681e5922bd 100644 --- a/src/common/logging/filter.h +++ b/src/common/logging/filter.h @@ -56,6 +56,9 @@ public: /// Matches class/level combination against the filter, returning true if it passed. bool CheckMessage(Class log_class, Level level) const; + /// Returns true if any logging classes are set to debug + bool IsDebug() const; + private: std::array(Class::Count)> class_levels; }; diff --git a/src/core/core.cpp b/src/core/core.cpp index a4ad8a8178..e9aff1a123 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -51,8 +51,6 @@ namespace Core { -/*static*/ System System::s_instance; - template <> Core::System& Global() { return System::GetInstance(); @@ -70,6 +68,13 @@ Core::Timing& Global() { System::~System() = default; +void System::InitializeGlobalInstance() { + if (s_instance) { + std::abort(); + } + s_instance = std::unique_ptr(new System); +} + System::ResultStatus System::RunLoop(bool tight_loop) { status = ResultStatus::Success; if (!IsPoweredOn()) { diff --git a/src/core/core.h b/src/core/core.h index b29fc20e23..b85febf263 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -74,9 +74,14 @@ public: * @returns Reference to the instance of the System singleton class. */ [[nodiscard]] static System& GetInstance() { - return s_instance; + if (!s_instance) { + std::abort(); + } + return *s_instance; } + static void InitializeGlobalInstance(); + /// Enumeration representing the return values of the System Initialize and Load process. enum class ResultStatus : u32 { Success, ///< Succeeded @@ -392,7 +397,7 @@ private: std::unique_ptr exclusive_monitor; private: - static System s_instance; + inline static std::unique_ptr s_instance; std::atomic_bool is_powered_on{}; diff --git a/src/dedicated_room/citra-room.cpp b/src/dedicated_room/citra-room.cpp index 4e68b34963..78fef0e906 100644 --- a/src/dedicated_room/citra-room.cpp +++ b/src/dedicated_room/citra-room.cpp @@ -150,17 +150,8 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin } static void InitializeLogging(const std::string& log_file) { - using namespace Common; - - Log::AddBackend(std::make_unique()); - - const std::string& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); - FileUtil::CreateFullPath(log_dir); - Log::AddBackend(std::make_unique(log_dir + log_file)); - -#ifdef _WIN32 - Log::AddBackend(std::make_unique()); -#endif + Common::Log::Initialize(); + Common::Log::SetColorConsoleBackendEnabled(true); } /// Application entry point diff --git a/src/tests/common/param_package.cpp b/src/tests/common/param_package.cpp index 75442cea40..b2a349d50e 100644 --- a/src/tests/common/param_package.cpp +++ b/src/tests/common/param_package.cpp @@ -4,11 +4,13 @@ #include #include +#include "common/logging/backend.h" #include "common/param_package.h" namespace Common { TEST_CASE("ParamPackage", "[common]") { + Common::Log::DisableLoggingInTests(); ParamPackage original{ {"abc", "xyz"}, {"def", "42"}, From a8340395a337d7f4de3686f9238e7f9fae6c8d19 Mon Sep 17 00:00:00 2001 From: yzct12345 <87620833+yzct12345@users.noreply.github.com> Date: Fri, 13 Aug 2021 18:58:35 +0000 Subject: [PATCH 10/25] logging: Display backtrace on crash This implements backtraces so we don't have to tell users how to use gdb anymore. This prints a backtrace after abort or segfault is detected. It also fixes the log getting cut off with the last line containing only a bracket. This change lets us know what caused a crash not just what happened the few seconds before it. I only know how to add support for Linux with GCC. Also this doesn't work outside of C/C++ such as in dynarmic or certain parts of graphics drivers. The good thing is that it'll try and just crash again but the stack frames are still there so the core dump will work just like before. --- src/common/CMakeLists.txt | 3 + src/common/logging/backend.cpp | 113 ++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 9063329513..cb5a432dc0 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -173,3 +173,6 @@ endif() if (CITRA_USE_PRECOMPILED_HEADERS) target_precompile_headers(citra_common PRIVATE precompiled_headers.h) endif() +if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux" AND CMAKE_CXX_COMPILER_ID STREQUAL GNU) + target_link_libraries(citra_common PRIVATE backtrace) +endif() diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 60d87b1b8a..58cffa33bc 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -12,6 +12,15 @@ #else #define _SH_DENYWR 0 #endif + +#if defined(__linux__) && defined(__GNUG__) && !defined(__clang__) +#define BOOST_STACKTRACE_USE_BACKTRACE +#include +#undef BOOST_STACKTRACE_USE_BACKTRACE +#include +#define CITRA_LINUX_GCC_BACKTRACE +#endif + #include "common/file_util.h" #include "common/literals.h" #include "common/settings.h" @@ -170,6 +179,14 @@ public: bool initialization_in_progress_suppress_logging = false; +#ifdef CITRA_LINUX_GCC_BACKTRACE +[[noreturn]] void SleepForever() { + while (true) { + pause(); + } +} +#endif + /** * Static state as a singleton. */ @@ -240,9 +257,66 @@ private: while (max_logs_to_write-- && message_queue.Pop(entry)) { write_logs(); } - })} {} + })} { +#ifdef CITRA_LINUX_GCC_BACKTRACE + int waker_pipefd[2]; + int done_printing_pipefd[2]; + if (pipe2(waker_pipefd, O_CLOEXEC) || pipe2(done_printing_pipefd, O_CLOEXEC)) { + abort(); + } + backtrace_thread_waker_fd = waker_pipefd[1]; + backtrace_done_printing_fd = done_printing_pipefd[0]; + std::thread([this, wait_fd = waker_pipefd[0], done_fd = done_printing_pipefd[1]] { + Common::SetCurrentThreadName("citra:Crash"); + for (u8 ignore = 0; read(wait_fd, &ignore, 1) != 1;) + ; + const int sig = received_signal; + if (sig <= 0) { + abort(); + } + StopBackendThread(); + const auto signal_entry = + CreateEntry(Class::Log, Level::Critical, "?", 0, "?", + fmt::vformat("Received signal {}", fmt::make_format_args(sig))); + ForEachBackend([&signal_entry](Backend& backend) { + backend.EnableForStacktrace(); + backend.Write(signal_entry); + }); + const auto backtrace = + boost::stacktrace::stacktrace::from_dump(backtrace_storage.data(), 4096); + for (const auto& frame : backtrace.as_vector()) { + auto line = boost::stacktrace::detail::to_string(&frame, 1); + if (line.empty()) { + abort(); + } + line.pop_back(); // Remove newline + const auto frame_entry = + CreateEntry(Class::Log, Level::Critical, "?", 0, "?", line); + ForEachBackend([&frame_entry](Backend& backend) { backend.Write(frame_entry); }); + } + using namespace std::literals; + const auto rip_entry = CreateEntry(Class::Log, Level::Critical, "?", 0, "?", "RIP"s); + ForEachBackend([&rip_entry](Backend& backend) { + backend.Write(rip_entry); + backend.Flush(); + }); + for (const u8 anything = 0; write(done_fd, &anything, 1) != 1;) + ; + // Abort on original thread to help debugging + SleepForever(); + }).detach(); + signal(SIGSEGV, &HandleSignal); + signal(SIGABRT, &HandleSignal); +#endif + } ~Impl() { +#ifdef CITRA_LINUX_GCC_BACKTRACE + if (int zero_or_ignore = 0; + !received_signal.compare_exchange_strong(zero_or_ignore, SIGKILL)) { + SleepForever(); + } +#endif StopBackendThread(); } @@ -281,6 +355,36 @@ private: delete ptr; } +#ifdef CITRA_LINUX_GCC_BACKTRACE + [[noreturn]] static void HandleSignal(int sig) { + signal(SIGABRT, SIG_DFL); + signal(SIGSEGV, SIG_DFL); + if (sig <= 0) { + abort(); + } + instance->InstanceHandleSignal(sig); + } + + [[noreturn]] void InstanceHandleSignal(int sig) { + if (int zero_or_ignore = 0; !received_signal.compare_exchange_strong(zero_or_ignore, sig)) { + if (received_signal == SIGKILL) { + abort(); + } + SleepForever(); + } + // Don't restart like boost suggests. We want to append to the log file and not lose dynamic + // symbols. This may segfault if it unwinds outside C/C++ code but we'll just have to fall + // back to core dumps. + boost::stacktrace::safe_dump_to(backtrace_storage.data(), 4096); + std::atomic_thread_fence(std::memory_order_seq_cst); + for (const int anything = 0; write(backtrace_thread_waker_fd, &anything, 1) != 1;) + ; + for (u8 ignore = 0; read(backtrace_done_printing_fd, &ignore, 1) != 1;) + ; + abort(); + } +#endif + static inline std::unique_ptr instance{nullptr, Deleter}; Filter filter; @@ -291,6 +395,13 @@ private: std::thread backend_thread; MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; + +#ifdef CITRA_LINUX_GCC_BACKTRACE + std::atomic_int received_signal{0}; + std::array backtrace_storage{}; + int backtrace_thread_waker_fd; + int backtrace_done_printing_fd; +#endif }; } // namespace From 98e9f4c32e196a8d431d93a55058a92df7810ad0 Mon Sep 17 00:00:00 2001 From: ameerj <52414509+ameerj@users.noreply.github.com> Date: Sat, 24 Jun 2023 03:05:04 +0300 Subject: [PATCH 11/25] logging: Fix log filter during initialization The log filter was being ignored on initialization due to the logging instance being initialized before the config instance, so the log filter was set to its default value. This fixes that oversight, along with using descriptive exceptions instead of abort() calls. --- src/citra_qt/main.cpp | 2 +- src/common/logging/backend.cpp | 8 +++++--- src/core/core.cpp | 10 +++++++++- src/core/core.h | 7 +------ 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 71f2937851..ee1102a218 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -171,6 +171,7 @@ static QString PrettyProductName() { GMainWindow::GMainWindow(Core::System& system_) : ui{std::make_unique()}, system{system_}, movie{Core::Movie::GetInstance()}, config{std::make_unique()}, emu_thread{nullptr} { + Common::Log::Initialize(); Debugger::ToggleConsole(); Settings::LogSettings(); @@ -2853,7 +2854,6 @@ static Qt::HighDpiScaleFactorRoundingPolicy GetHighDpiRoundingPolicy() { } int main(int argc, char* argv[]) { - Common::Log::Initialize(); Common::DetachedTasks detached_tasks; MicroProfileOnThreadCreate("Frontend"); SCOPE_EXIT({ MicroProfileShutdown(); }); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 58cffa33bc..c835a0d852 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include "common/common_paths.h" @@ -177,7 +178,7 @@ public: }; #endif -bool initialization_in_progress_suppress_logging = false; +bool initialization_in_progress_suppress_logging = true; #ifdef CITRA_LINUX_GCC_BACKTRACE [[noreturn]] void SleepForever() { @@ -194,14 +195,15 @@ class Impl { public: static Impl& Instance() { if (!instance) { - abort(); + throw std::runtime_error("Using Logging instance before its initialization"); } return *instance; } static void Initialize() { if (instance) { - abort(); + LOG_WARNING(Log, "Reinitializing logging backend"); + return; } initialization_in_progress_suppress_logging = true; const auto& log_dir = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); diff --git a/src/core/core.cpp b/src/core/core.cpp index e9aff1a123..4d4d567e96 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include #include #include @@ -68,9 +69,16 @@ Core::Timing& Global() { System::~System() = default; +System& System::GetInstance() { + if (!s_instance) { + throw std::runtime_error("Using System instance before its initialization"); + } + return *s_instance; +} + void System::InitializeGlobalInstance() { if (s_instance) { - std::abort(); + throw std::runtime_error("Reinitializing Global System instance."); } s_instance = std::unique_ptr(new System); } diff --git a/src/core/core.h b/src/core/core.h index b85febf263..9bc49c9c30 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -73,12 +73,7 @@ public: * Gets the instance of the System singleton class. * @returns Reference to the instance of the System singleton class. */ - [[nodiscard]] static System& GetInstance() { - if (!s_instance) { - std::abort(); - } - return *s_instance; - } + [[nodiscard]] static System& GetInstance(); static void InitializeGlobalInstance(); From 8f51dd9513d9f4202da5e8eacc8d3b9a0b4ab408 Mon Sep 17 00:00:00 2001 From: ameerj <52414509+ameerj@users.noreply.github.com> Date: Sat, 24 Jun 2023 10:01:54 +0300 Subject: [PATCH 12/25] common/logging: Move Log::Entry declaration to a separate header This reduces the load of requiring to include std::chrono in all files which include log.h --- src/common/CMakeLists.txt | 1 + src/common/logging/backend.cpp | 6 +++++- src/common/logging/log.h | 2 ++ src/common/logging/log_entry.h | 28 +++++++++++++++++++++++++++ src/common/logging/text_formatter.cpp | 1 + src/common/logging/types.h | 17 ---------------- 6 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 src/common/logging/log_entry.h diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index cb5a432dc0..e3df8b3081 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -84,6 +84,7 @@ add_library(citra_common STATIC logging/filter.h logging/formatter.h logging/log.h + logging/log_entry.h logging/text_formatter.cpp logging/text_formatter.h logging/types.h diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index c835a0d852..a672943fa4 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -6,7 +6,9 @@ #include #include #include -#include "common/common_paths.h" + +#include + #ifdef _WIN32 #include // For _SH_DENYWR #include // For OutputDebugStringW @@ -22,12 +24,14 @@ #define CITRA_LINUX_GCC_BACKTRACE #endif +#include "common/common_paths.h" #include "common/file_util.h" #include "common/literals.h" #include "common/settings.h" #include "common/thread.h" #include "common/logging/backend.h" #include "common/logging/log.h" +#include "common/logging/log_entry.h" #include "common/logging/text_formatter.h" #include "common/string_util.h" #include "common/threadsafe_queue.h" diff --git a/src/common/logging/log.h b/src/common/logging/log.h index ab596d08f7..59269af3ea 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -6,6 +6,8 @@ #include #include +#include + #include "common/logging/formatter.h" #include "common/logging/types.h" diff --git a/src/common/logging/log_entry.h b/src/common/logging/log_entry.h new file mode 100644 index 0000000000..dd6f448418 --- /dev/null +++ b/src/common/logging/log_entry.h @@ -0,0 +1,28 @@ +// Copyright 2021 yuzu Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +#include + +#include "common/logging/types.h" + +namespace Common::Log { + +/** + * A log entry. Log entries are store in a structured format to permit more varied output + * formatting on different frontends, as well as facilitating filtering and aggregation. + */ +struct Entry { + std::chrono::microseconds timestamp; + Class log_class{}; + Level log_level{}; + const char* filename = nullptr; + unsigned int line_num = 0; + std::string function; + std::string message; + bool final_entry = false; +}; + +} // namespace Common::Log diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index 9942dd76b3..27ec5164b6 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -15,6 +15,7 @@ #include "common/common_funcs.h" #include "common/logging/filter.h" #include "common/logging/log.h" +#include "common/logging/log_entry.h" #include "common/logging/text_formatter.h" #include "common/string_util.h" diff --git a/src/common/logging/types.h b/src/common/logging/types.h index 8dd0b74441..e8fc3feade 100644 --- a/src/common/logging/types.h +++ b/src/common/logging/types.h @@ -4,8 +4,6 @@ #pragma once -#include - #include "common/common_types.h" namespace Common::Log { @@ -105,19 +103,4 @@ enum class Class : u8 { Count ///< Total number of logging classes }; -/** - * A log entry. Log entries are store in a structured format to permit more varied output - * formatting on different frontends, as well as facilitating filtering and aggregation. - */ -struct Entry { - std::chrono::microseconds timestamp; - Class log_class{}; - Level log_level{}; - const char* filename = nullptr; - unsigned int line_num = 0; - std::string function; - std::string message; - bool final_entry = false; -}; - } // namespace Common::Log From aa39430e2c4ff506aaf83e07ec2fcd14fd873527 Mon Sep 17 00:00:00 2001 From: ameerj <52414509+ameerj@users.noreply.github.com> Date: Sat, 24 Jun 2023 10:08:18 +0300 Subject: [PATCH 13/25] common/logging: Reduce scope of fmt include --- src/common/logging/formatter.h | 2 +- src/common/param_package.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/logging/formatter.h b/src/common/logging/formatter.h index ad6adb1435..e27850db1a 100644 --- a/src/common/logging/formatter.h +++ b/src/common/logging/formatter.h @@ -5,7 +5,7 @@ #pragma once #include -#include +#include // adapted from https://github.com/fmtlib/fmt/issues/2704 // a generic formatter for enum classes diff --git a/src/common/param_package.cpp b/src/common/param_package.cpp index bbdde251fa..188f45aa61 100644 --- a/src/common/param_package.cpp +++ b/src/common/param_package.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include "common/logging/log.h" From a1443356f1d2b9cb023c8184e3f1b66c8db978cd Mon Sep 17 00:00:00 2001 From: ameerj <52414509+ameerj@users.noreply.github.com> Date: Sat, 24 Jun 2023 10:11:54 +0300 Subject: [PATCH 14/25] threadsafe_queue: Add std::stop_token overload to PopWait Useful for jthreads which make use of the threadsafe queues. --- src/common/logging/backend.cpp | 4 ++-- src/common/threadsafe_queue.h | 27 +++++++++++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index a672943fa4..19efa5dcb4 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -27,13 +27,13 @@ #include "common/common_paths.h" #include "common/file_util.h" #include "common/literals.h" -#include "common/settings.h" -#include "common/thread.h" #include "common/logging/backend.h" #include "common/logging/log.h" #include "common/logging/log_entry.h" #include "common/logging/text_formatter.h" +#include "common/settings.h" #include "common/string_util.h" +#include "common/thread.h" #include "common/threadsafe_queue.h" namespace Common::Log { diff --git a/src/common/threadsafe_queue.h b/src/common/threadsafe_queue.h index 5584229a62..d69ab12891 100644 --- a/src/common/threadsafe_queue.h +++ b/src/common/threadsafe_queue.h @@ -13,8 +13,10 @@ #include #include +#include "common/polyfill_thread.h" + namespace Common { -template +template class SPSCQueue { public: SPSCQueue() { @@ -93,6 +95,19 @@ public: return t; } + T PopWait(std::stop_token stop_token) { + if (Empty()) { + std::unique_lock lock{cv_mutex}; + CondvarWait(cv, lock, stop_token, [this] { return !Empty(); }); + } + if (stop_token.stop_requested()) { + return T{}; + } + T t; + Pop(t); + return t; + } + // not thread-safe void Clear() { size.store(0); @@ -121,13 +136,13 @@ private: ElementPtr* read_ptr; std::atomic_size_t size{0}; std::mutex cv_mutex; - std::condition_variable cv; + std::conditional_t cv; }; // a simple thread-safe, // single reader, multiple writer queue -template +template class MPSCQueue { public: [[nodiscard]] std::size_t Size() const { @@ -160,13 +175,17 @@ public: return spsc_queue.PopWait(); } + T PopWait(std::stop_token stop_token) { + return spsc_queue.PopWait(stop_token); + } + // not thread-safe void Clear() { spsc_queue.Clear(); } private: - SPSCQueue spsc_queue; + SPSCQueue spsc_queue; std::mutex write_lock; }; } // namespace Common From 637ade3b25bb2e37b94989739d8f986cf64c5db4 Mon Sep 17 00:00:00 2001 From: yzct12345 <87620833+yzct12345@users.noreply.github.com> Date: Fri, 13 Aug 2021 19:22:51 +0000 Subject: [PATCH 15/25] threadsafe_queue: Fix deadlock This fixes a lost wakeup in SPSCQueue. If the reader is in just the right position, the writer's notification will be lost and this will be a problem if the writer then does something to wait on the reader. This was discovered to affect my upcoming stacktrace PR. I don't think any performance decrease will be noticeable because an uncontended mutex is smart enough to skip the syscall. This PR might also resolve some rare deadlocks but I don't know of any examples. --- src/common/threadsafe_queue.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/common/threadsafe_queue.h b/src/common/threadsafe_queue.h index d69ab12891..2754d585c8 100644 --- a/src/common/threadsafe_queue.h +++ b/src/common/threadsafe_queue.h @@ -48,15 +48,13 @@ public: ElementPtr* new_ptr = new ElementPtr(); write_ptr->next.store(new_ptr, std::memory_order_release); write_ptr = new_ptr; + ++size; - const size_t previous_size{size++}; - - // Acquire the mutex and then immediately release it as a fence. + // cv_mutex must be held or else there will be a missed wakeup if the other thread is in the + // line before cv.wait // TODO(bunnei): This can be replaced with C++20 waitable atomics when properly supported. // See discussion on https://github.com/yuzu-emu/yuzu/pull/3173 for details. - if (previous_size == 0) { - std::lock_guard lock{cv_mutex}; - } + std::lock_guard lock{cv_mutex}; cv.notify_one(); } From fe027a96fb183e644b2a3f22d79356228a0fc1f1 Mon Sep 17 00:00:00 2001 From: Merry Date: Thu, 7 Apr 2022 19:30:55 +0100 Subject: [PATCH 16/25] common: Replace lock_guard with scoped_lock --- src/common/threadsafe_queue.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/threadsafe_queue.h b/src/common/threadsafe_queue.h index 2754d585c8..8217e2bda4 100644 --- a/src/common/threadsafe_queue.h +++ b/src/common/threadsafe_queue.h @@ -54,7 +54,7 @@ public: // line before cv.wait // TODO(bunnei): This can be replaced with C++20 waitable atomics when properly supported. // See discussion on https://github.com/yuzu-emu/yuzu/pull/3173 for details. - std::lock_guard lock{cv_mutex}; + std::scoped_lock lock{cv_mutex}; cv.notify_one(); } @@ -157,7 +157,7 @@ public: template void Push(Arg&& t) { - std::lock_guard lock{write_lock}; + std::scoped_lock lock{write_lock}; spsc_queue.Push(t); } From 197c1adcbab034584a443742d3f2a56beae9e4d5 Mon Sep 17 00:00:00 2001 From: Levi Behunin Date: Sat, 24 Jun 2023 11:41:53 +0300 Subject: [PATCH 17/25] 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 From ae6fda8638d9abd2e02d8d4b57c8e5494e0847e3 Mon Sep 17 00:00:00 2001 From: Wunkolo Date: Sat, 24 Jun 2023 11:43:10 +0300 Subject: [PATCH 18/25] logging: Convert backend_thread into an std::jthread Was getting an unhandled `invalid_argument` [exception](https://en.cppreference.com/w/cpp/thread/thread/join) during shutdown on my linux machine. This removes the need for a `StopBackendThread` function entirely since `jthread` [automatically handles both checking if the thread is joinable and stopping the token before attempting to join](https://en.cppreference.com/w/cpp/thread/jthread/~jthread) in the case that `StartBackendThread` was never called. --- src/common/logging/backend.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index ad01c24374..d342ff466f 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -265,7 +265,8 @@ private: if (sig <= 0) { abort(); } - StopBackendThread(); + backend_thread.request_stop(); + backend_thread.join(); const auto signal_entry = CreateEntry(Class::Log, Level::Critical, "?", 0, "?", fmt::vformat("Received signal {}", fmt::make_format_args(sig))); @@ -308,18 +309,17 @@ private: SleepForever(); } #endif - StopBackendThread(); } void StartBackendThread() { - backend_thread = std::thread([this] { + backend_thread = std::jthread([this](std::stop_token stop_token) { 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()); + while (!stop_token.stop_requested()) { + entry = message_queue.PopWait(stop_token); if (entry.filename != nullptr) { write_logs(); } @@ -333,11 +333,6 @@ private: }); } - void StopBackendThread() { - 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 { using std::chrono::duration_cast; @@ -402,8 +397,7 @@ private: ColorConsoleBackend color_console_backend{}; FileBackend file_backend; - std::stop_source stop; - std::thread backend_thread; + std::jthread backend_thread; MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; From e112421db8a2a6bf0e270a053c093ce170d104df Mon Sep 17 00:00:00 2001 From: Merry Date: Sat, 24 Jun 2023 11:44:13 +0300 Subject: [PATCH 19/25] backend: Ensure backend_thread is destructed before message_queue Ensures that stop_token signals that stop has been requested before destruction of conditional_variable --- src/common/logging/backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index d342ff466f..55bf4c0f5a 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -397,9 +397,9 @@ private: ColorConsoleBackend color_console_backend{}; FileBackend file_backend; - std::jthread backend_thread; MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; + std::jthread backend_thread; #ifdef CITRA_LINUX_GCC_BACKTRACE std::atomic_int received_signal{0}; From 52b9007fcfb397aa95cd15aaaeccfe689ef3b107 Mon Sep 17 00:00:00 2001 From: ameerj <52414509+ameerj@users.noreply.github.com> Date: Sat, 24 Jun 2023 11:46:33 +0300 Subject: [PATCH 20/25] common: Reduce unused includes --- src/audio_core/hle/wmf_decoder_utils.h | 1 + src/common/dynamic_library/fdk-aac.cpp | 2 ++ src/common/dynamic_library/ffmpeg.cpp | 2 ++ src/common/file_util.cpp | 1 + src/common/logging/backend.cpp | 2 -- src/common/logging/backend.h | 1 - src/common/logging/filter.cpp | 2 ++ src/common/logging/text_formatter.cpp | 2 -- src/common/logging/text_formatter.h | 1 - src/core/hle/service/cfg/cfg.cpp | 1 + src/core/hle/service/ir/extra_hid.cpp | 1 + src/core/hle/service/ir/ir_user.cpp | 1 + src/core/movie.cpp | 1 + src/core/savestate.cpp | 1 + 14 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/audio_core/hle/wmf_decoder_utils.h b/src/audio_core/hle/wmf_decoder_utils.h index 9925b83abf..77a12bef58 100644 --- a/src/audio_core/hle/wmf_decoder_utils.h +++ b/src/audio_core/hle/wmf_decoder_utils.h @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #pragma once +#include #include #include #include diff --git a/src/common/dynamic_library/fdk-aac.cpp b/src/common/dynamic_library/fdk-aac.cpp index 20dedfd07b..48f8ca2cc0 100644 --- a/src/common/dynamic_library/fdk-aac.cpp +++ b/src/common/dynamic_library/fdk-aac.cpp @@ -2,6 +2,8 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include + #include "common/dynamic_library/dynamic_library.h" #include "common/dynamic_library/fdk-aac.h" #include "common/logging/log.h" diff --git a/src/common/dynamic_library/ffmpeg.cpp b/src/common/dynamic_library/ffmpeg.cpp index c9c1af33a9..709f887a52 100644 --- a/src/common/dynamic_library/ffmpeg.cpp +++ b/src/common/dynamic_library/ffmpeg.cpp @@ -2,6 +2,8 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include + #include "common/dynamic_library/dynamic_library.h" #include "common/dynamic_library/ffmpeg.h" #include "common/logging/log.h" diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 3a68e73494..11ff6bcf57 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include "common/assert.h" diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 55bf4c0f5a..deb20bb3b5 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -3,8 +3,6 @@ // Refer to the license.txt file included. #include -#include -#include #include diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index bf785f402e..a0e80fe3c1 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -4,7 +4,6 @@ #pragma once -#include #include "common/logging/filter.h" namespace Common::Log { diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index 6213e3f49b..c469993e8b 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -3,6 +3,8 @@ // Refer to the license.txt file included. #include + +#include "common/assert.h" #include "common/logging/filter.h" #include "common/string_util.h" diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index 27ec5164b6..753e5003e6 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -12,12 +12,10 @@ #endif #include "common/assert.h" -#include "common/common_funcs.h" #include "common/logging/filter.h" #include "common/logging/log.h" #include "common/logging/log_entry.h" #include "common/logging/text_formatter.h" -#include "common/string_util.h" namespace Common::Log { diff --git a/src/common/logging/text_formatter.h b/src/common/logging/text_formatter.h index a15b41c42a..4ab9e18f9b 100644 --- a/src/common/logging/text_formatter.h +++ b/src/common/logging/text_formatter.h @@ -4,7 +4,6 @@ #pragma once -#include #include namespace Common::Log { diff --git a/src/core/hle/service/cfg/cfg.cpp b/src/core/hle/service/cfg/cfg.cpp index 3b2c6e1130..43d2e9ed8c 100644 --- a/src/core/hle/service/cfg/cfg.cpp +++ b/src/core/hle/service/cfg/cfg.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include "common/archives.h" #include "common/file_util.h" #include "common/logging/log.h" diff --git a/src/core/hle/service/ir/extra_hid.cpp b/src/core/hle/service/ir/extra_hid.cpp index eff81c7111..4709f70cf4 100644 --- a/src/core/hle/service/ir/extra_hid.cpp +++ b/src/core/hle/service/ir/extra_hid.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include "common/alignment.h" #include "common/settings.h" #include "common/string_util.h" diff --git a/src/core/hle/service/ir/ir_user.cpp b/src/core/hle/service/ir/ir_user.cpp index 1900db9029..81d631f946 100644 --- a/src/core/hle/service/ir/ir_user.cpp +++ b/src/core/hle/service/ir/ir_user.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include "common/string_util.h" #include "common/swap.h" #include "core/core.h" diff --git a/src/core/movie.cpp b/src/core/movie.cpp index c2d085d29b..83264ea357 100644 --- a/src/core/movie.cpp +++ b/src/core/movie.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "common/bit_field.h" #include "common/common_types.h" #include "common/file_util.h" diff --git a/src/core/savestate.cpp b/src/core/savestate.cpp index 4c88eade4e..e791a27f70 100644 --- a/src/core/savestate.cpp +++ b/src/core/savestate.cpp @@ -4,6 +4,7 @@ #include #include +#include #include "common/archives.h" #include "common/logging/log.h" #include "common/scm_rev.h" From 0ddb09527302e5d707072e31ea8415ac3b6d3461 Mon Sep 17 00:00:00 2001 From: Morph <39850852+Morph1984@users.noreply.github.com> Date: Sat, 24 Jun 2023 11:54:18 +0300 Subject: [PATCH 21/25] logging: Make use of bounded queue --- src/common/CMakeLists.txt | 1 + src/common/bounded_threadsafe_queue.h | 250 ++++++++++++++++++++++++++ src/common/logging/backend.cpp | 16 +- 3 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 src/common/bounded_threadsafe_queue.h diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index e3df8b3081..4e3500a389 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -57,6 +57,7 @@ add_library(citra_common STATIC detached_tasks.h bit_field.h bit_set.h + bounded_threadsafe_queue.h cityhash.cpp cityhash.h color.h diff --git a/src/common/bounded_threadsafe_queue.h b/src/common/bounded_threadsafe_queue.h new file mode 100644 index 0000000000..04d1905727 --- /dev/null +++ b/src/common/bounded_threadsafe_queue.h @@ -0,0 +1,250 @@ +// Copyright 2023 yuzu Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +#include +#include +#include +#include +#include + +#include "common/polyfill_thread.h" + +namespace Common { + +namespace detail { +constexpr size_t DefaultCapacity = 0x1000; +} // namespace detail + +template +class SPSCQueue { + static_assert((Capacity & (Capacity - 1)) == 0, "Capacity must be a power of two."); + +public: + template + bool TryEmplace(Args&&... args) { + return Emplace(std::forward(args)...); + } + + template + void EmplaceWait(Args&&... args) { + Emplace(std::forward(args)...); + } + + bool TryPop(T& t) { + return Pop(t); + } + + void PopWait(T& t) { + Pop(t); + } + + void PopWait(T& t, std::stop_token stop_token) { + Pop(t, stop_token); + } + + T PopWait() { + T t; + Pop(t); + return t; + } + + T PopWait(std::stop_token stop_token) { + T t; + Pop(t, stop_token); + return t; + } + +private: + enum class PushMode { + Try, + Wait, + Count, + }; + + enum class PopMode { + Try, + Wait, + WaitWithStopToken, + Count, + }; + + template + bool Emplace(Args&&... args) { + const size_t write_index = m_write_index.load(std::memory_order::relaxed); + + if constexpr (Mode == PushMode::Try) { + // Check if we have free slots to write to. + if ((write_index - m_read_index.load(std::memory_order::acquire)) == Capacity) { + return false; + } + } else if constexpr (Mode == PushMode::Wait) { + // Wait until we have free slots to write to. + std::unique_lock lock{producer_cv_mutex}; + producer_cv.wait(lock, [this, write_index] { + return (write_index - m_read_index.load(std::memory_order::acquire)) < Capacity; + }); + } else { + static_assert(Mode < PushMode::Count, "Invalid PushMode."); + } + + // Determine the position to write to. + const size_t pos = write_index % Capacity; + + // Emplace into the queue. + std::construct_at(std::addressof(m_data[pos]), std::forward(args)...); + + // Increment the write index. + ++m_write_index; + + // Notify the consumer that we have pushed into the queue. + std::scoped_lock lock{consumer_cv_mutex}; + consumer_cv.notify_one(); + + return true; + } + + template + bool Pop(T& t, [[maybe_unused]] std::stop_token stop_token = {}) { + const size_t read_index = m_read_index.load(std::memory_order::relaxed); + + if constexpr (Mode == PopMode::Try) { + // Check if the queue is empty. + if (read_index == m_write_index.load(std::memory_order::acquire)) { + return false; + } + } else if constexpr (Mode == PopMode::Wait) { + // Wait until the queue is not empty. + std::unique_lock lock{consumer_cv_mutex}; + consumer_cv.wait(lock, [this, read_index] { + return read_index != m_write_index.load(std::memory_order::acquire); + }); + } else if constexpr (Mode == PopMode::WaitWithStopToken) { + // Wait until the queue is not empty. + std::unique_lock lock{consumer_cv_mutex}; + Common::CondvarWait(consumer_cv, lock, stop_token, [this, read_index] { + return read_index != m_write_index.load(std::memory_order::acquire); + }); + if (stop_token.stop_requested()) { + return false; + } + } else { + static_assert(Mode < PopMode::Count, "Invalid PopMode."); + } + + // Determine the position to read from. + const size_t pos = read_index % Capacity; + + // Pop the data off the queue, moving it. + t = std::move(m_data[pos]); + + // Increment the read index. + ++m_read_index; + + // Notify the producer that we have popped off the queue. + std::scoped_lock lock{producer_cv_mutex}; + producer_cv.notify_one(); + + return true; + } + + alignas(128) std::atomic_size_t m_read_index{0}; + alignas(128) std::atomic_size_t m_write_index{0}; + + std::array m_data; + + std::condition_variable_any producer_cv; + std::mutex producer_cv_mutex; + std::condition_variable_any consumer_cv; + std::mutex consumer_cv_mutex; +}; + +template +class MPSCQueue { +public: + template + bool TryEmplace(Args&&... args) { + std::scoped_lock lock{write_mutex}; + return spsc_queue.TryEmplace(std::forward(args)...); + } + + template + void EmplaceWait(Args&&... args) { + std::scoped_lock lock{write_mutex}; + spsc_queue.EmplaceWait(std::forward(args)...); + } + + bool TryPop(T& t) { + return spsc_queue.TryPop(t); + } + + void PopWait(T& t) { + spsc_queue.PopWait(t); + } + + void PopWait(T& t, std::stop_token stop_token) { + spsc_queue.PopWait(t, stop_token); + } + + T PopWait() { + return spsc_queue.PopWait(); + } + + T PopWait(std::stop_token stop_token) { + return spsc_queue.PopWait(stop_token); + } + +private: + SPSCQueue spsc_queue; + std::mutex write_mutex; +}; + +template +class MPMCQueue { +public: + template + bool TryEmplace(Args&&... args) { + std::scoped_lock lock{write_mutex}; + return spsc_queue.TryEmplace(std::forward(args)...); + } + + template + void EmplaceWait(Args&&... args) { + std::scoped_lock lock{write_mutex}; + spsc_queue.EmplaceWait(std::forward(args)...); + } + + bool TryPop(T& t) { + std::scoped_lock lock{read_mutex}; + return spsc_queue.TryPop(t); + } + + void PopWait(T& t) { + std::scoped_lock lock{read_mutex}; + spsc_queue.PopWait(t); + } + + void PopWait(T& t, std::stop_token stop_token) { + std::scoped_lock lock{read_mutex}; + spsc_queue.PopWait(t, stop_token); + } + + T PopWait() { + std::scoped_lock lock{read_mutex}; + return spsc_queue.PopWait(); + } + + T PopWait(std::stop_token stop_token) { + std::scoped_lock lock{read_mutex}; + return spsc_queue.PopWait(stop_token); + } + +private: + SPSCQueue spsc_queue; + std::mutex write_mutex; + std::mutex read_mutex; +}; + +} // namespace Common diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index deb20bb3b5..36f4a3f243 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -21,6 +21,7 @@ #define CITRA_LINUX_GCC_BACKTRACE #endif +#include "common/bounded_threadsafe_queue.h" #include "common/common_paths.h" #include "common/file_util.h" #include "common/literals.h" @@ -32,7 +33,6 @@ #include "common/settings.h" #include "common/string_util.h" #include "common/thread.h" -#include "common/threadsafe_queue.h" namespace Common::Log { @@ -237,11 +237,11 @@ public: void PushEntry(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, std::string message) { - if (!filter.CheckMessage(log_class, log_level)) + if (!filter.CheckMessage(log_class, log_level)) { return; - const Entry& entry = - CreateEntry(log_class, log_level, filename, line_num, function, std::move(message)); - message_queue.Push(entry); + } + message_queue.EmplaceWait( + CreateEntry(log_class, log_level, filename, line_num, function, std::move(message))); } private: @@ -317,7 +317,7 @@ private: ForEachBackend([&entry](Backend& backend) { backend.Write(entry); }); }; while (!stop_token.stop_requested()) { - entry = message_queue.PopWait(stop_token); + message_queue.PopWait(entry, stop_token); if (entry.filename != nullptr) { write_logs(); } @@ -325,7 +325,7 @@ private: // 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)) { + while (max_logs_to_write-- && message_queue.TryPop(entry)) { write_logs(); } }); @@ -395,7 +395,7 @@ private: ColorConsoleBackend color_console_backend{}; FileBackend file_backend; - MPSCQueue message_queue{}; + MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; std::jthread backend_thread; From 9c3e2d0f501dd0e688d8468095418c783563d9b5 Mon Sep 17 00:00:00 2001 From: GPUCode Date: Sat, 24 Jun 2023 18:25:47 +0300 Subject: [PATCH 22/25] externals: Update boost * To include stack traces --- externals/boost | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/externals/boost b/externals/boost index 32f5cd8ebb..700ae2eff3 160000 --- a/externals/boost +++ b/externals/boost @@ -1 +1 @@ -Subproject commit 32f5cd8ebb35b29ccb3860861bd285f80804bc85 +Subproject commit 700ae2eff3134792f09cea2b051666688b1d5b97 From ba98bf058adeb10c7935be55b1b439d7f86443df Mon Sep 17 00:00:00 2001 From: GPUCode Date: Sat, 24 Jun 2023 18:30:13 +0300 Subject: [PATCH 23/25] logging: Address some issues --- src/android/app/src/main/jni/id_cache.cpp | 6 ------ src/android/app/src/main/jni/native.cpp | 6 ++---- src/citra/citra.cpp | 1 - src/citra_qt/main.cpp | 1 - src/common/bounded_threadsafe_queue.h | 3 ++- src/common/file_util.cpp | 2 +- src/common/logging/backend.cpp | 6 ++++++ src/common/logging/formatter.h | 2 +- src/core/core.cpp | 16 ++-------------- src/core/core.h | 8 ++++---- src/dedicated_room/CMakeLists.txt | 2 +- 11 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/android/app/src/main/jni/id_cache.cpp b/src/android/app/src/main/jni/id_cache.cpp index c2aa048a86..e737d19ee5 100644 --- a/src/android/app/src/main/jni/id_cache.cpp +++ b/src/android/app/src/main/jni/id_cache.cpp @@ -180,12 +180,6 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) { if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION) != JNI_OK) return JNI_ERR; - // Initialize Logger - Log::Filter log_filter; - log_filter.ParseFilterString(Settings::values.log_filter.GetValue()); - Log::SetGlobalFilter(log_filter); - Log::AddBackend(std::make_unique()); - // Initialize misc classes s_savestate_info_class = reinterpret_cast( env->NewGlobalRef(env->FindClass("org/citra/citra_emu/NativeLibrary$SavestateInfo"))); diff --git a/src/android/app/src/main/jni/native.cpp b/src/android/app/src/main/jni/native.cpp index e9ea0cb375..d1071ad3b2 100644 --- a/src/android/app/src/main/jni/native.cpp +++ b/src/android/app/src/main/jni/native.cpp @@ -438,10 +438,8 @@ void Java_org_citra_citra_1emu_NativeLibrary_CreateConfigFile(JNIEnv* env, void Java_org_citra_citra_1emu_NativeLibrary_CreateLogFile(JNIEnv* env, [[maybe_unused]] jclass clazz) { - Log::RemoveBackend(Log::FileBackend::Name()); - FileUtil::CreateFullPath(FileUtil::GetUserPath(FileUtil::UserPath::LogDir)); - Log::AddBackend(std::make_unique( - FileUtil::GetUserPath(FileUtil::UserPath::LogDir) + LOG_FILE)); + Common::Log::Initialize(); + Common::Log::Start(); LOG_INFO(Frontend, "Logging backend initialised"); } diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index e3b71d26db..3a9640c6e8 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -330,7 +330,6 @@ int main(int argc, char** argv) { return -1; } - Core::System::InitializeGlobalInstance(); auto& system = Core::System::GetInstance(); auto& movie = Core::Movie::GetInstance(); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index cc76ecd8e2..7a16bbfd36 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -2880,7 +2880,6 @@ int main(int argc, char* argv[]) { // generating shaders setlocale(LC_ALL, "C"); - Core::System::InitializeGlobalInstance(); auto& system{Core::System::GetInstance()}; GMainWindow main_window(system); diff --git a/src/common/bounded_threadsafe_queue.h b/src/common/bounded_threadsafe_queue.h index 04d1905727..15c9c1db09 100644 --- a/src/common/bounded_threadsafe_queue.h +++ b/src/common/bounded_threadsafe_queue.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -94,7 +95,7 @@ private: const size_t pos = write_index % Capacity; // Emplace into the queue. - std::construct_at(std::addressof(m_data[pos]), std::forward(args)...); + new (std::addressof(m_data[pos])) T(std::forward(args)...); // Increment the write index. ++m_write_index; diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 11ff6bcf57..3fa335d357 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -8,9 +8,9 @@ #include #include #include -#include #include #include +#include #include "common/assert.h" #include "common/common_funcs.h" #include "common/common_paths.h" diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 36f4a3f243..d868b26094 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -352,6 +352,9 @@ private: lambda(static_cast(debugger_backend)); lambda(static_cast(color_console_backend)); lambda(static_cast(file_backend)); +#ifdef ANDROID + lambda(static_cast(lc_backend)); +#endif } static void Deleter(Impl* ptr) { @@ -394,6 +397,9 @@ private: DebuggerBackend debugger_backend{}; ColorConsoleBackend color_console_backend{}; FileBackend file_backend; +#ifdef ANDROID + LogcatBackend lc_backend{}; +#endif MPSCQueue message_queue{}; std::chrono::steady_clock::time_point time_origin{std::chrono::steady_clock::now()}; diff --git a/src/common/logging/formatter.h b/src/common/logging/formatter.h index e27850db1a..ad6adb1435 100644 --- a/src/common/logging/formatter.h +++ b/src/common/logging/formatter.h @@ -5,7 +5,7 @@ #pragma once #include -#include +#include // adapted from https://github.com/fmtlib/fmt/issues/2704 // a generic formatter for enum classes diff --git a/src/core/core.cpp b/src/core/core.cpp index 4d4d567e96..f3bbc12d73 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -52,6 +52,8 @@ namespace Core { +/*static*/ System System::s_instance; + template <> Core::System& Global() { return System::GetInstance(); @@ -69,20 +71,6 @@ Core::Timing& Global() { System::~System() = default; -System& System::GetInstance() { - if (!s_instance) { - throw std::runtime_error("Using System instance before its initialization"); - } - return *s_instance; -} - -void System::InitializeGlobalInstance() { - if (s_instance) { - throw std::runtime_error("Reinitializing Global System instance."); - } - s_instance = std::unique_ptr(new System); -} - System::ResultStatus System::RunLoop(bool tight_loop) { status = ResultStatus::Success; if (!IsPoweredOn()) { diff --git a/src/core/core.h b/src/core/core.h index 9bc49c9c30..b29fc20e23 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -73,9 +73,9 @@ public: * Gets the instance of the System singleton class. * @returns Reference to the instance of the System singleton class. */ - [[nodiscard]] static System& GetInstance(); - - static void InitializeGlobalInstance(); + [[nodiscard]] static System& GetInstance() { + return s_instance; + } /// Enumeration representing the return values of the System Initialize and Load process. enum class ResultStatus : u32 { @@ -392,7 +392,7 @@ private: std::unique_ptr exclusive_monitor; private: - inline static std::unique_ptr s_instance; + static System s_instance; std::atomic_bool is_powered_on{}; diff --git a/src/dedicated_room/CMakeLists.txt b/src/dedicated_room/CMakeLists.txt index 7f130a9a82..4df160a8d6 100644 --- a/src/dedicated_room/CMakeLists.txt +++ b/src/dedicated_room/CMakeLists.txt @@ -8,7 +8,7 @@ add_executable(citra-room create_target_directory_groups(citra-room) -target_link_libraries(citra-room PRIVATE citra_common network) +target_link_libraries(citra-room PRIVATE citra_common citra_core network) if (ENABLE_WEB_SERVICE) target_compile_definitions(citra-room PRIVATE -DENABLE_WEB_SERVICE) target_link_libraries(citra-room PRIVATE web_service) From 9527bfffed5ecc8c8c53e388e8d5905c9db6760f Mon Sep 17 00:00:00 2001 From: GPUCode Date: Fri, 30 Jun 2023 13:39:38 +0300 Subject: [PATCH 24/25] common: Remove dependency from core --- src/android/app/src/main/jni/native.cpp | 4 +- src/citra/citra.cpp | 2 +- .../configuration/configure_dialog.cpp | 9 ++- src/citra_qt/configuration/configure_dialog.h | 7 +- .../configuration/configure_per_game.cpp | 2 +- src/citra_qt/dumping/dumping_dialog.cpp | 7 +- src/citra_qt/dumping/dumping_dialog.h | 7 +- src/citra_qt/main.cpp | 16 ++--- src/common/CMakeLists.txt | 2 +- src/common/settings.cpp | 70 +------------------ src/common/settings.h | 1 - src/core/core.cpp | 69 ++++++++++++++++-- src/core/core.h | 3 + src/dedicated_room/CMakeLists.txt | 2 +- 14 files changed, 105 insertions(+), 96 deletions(-) diff --git a/src/android/app/src/main/jni/native.cpp b/src/android/app/src/main/jni/native.cpp index d1071ad3b2..ef2b62f2ea 100644 --- a/src/android/app/src/main/jni/native.cpp +++ b/src/android/app/src/main/jni/native.cpp @@ -141,7 +141,7 @@ static Core::System::ResultStatus RunCitra(const std::string& filepath) { app_loader->ReadProgramId(program_id); GameSettings::LoadOverrides(program_id); } - Settings::Apply(); + system.ApplySettings(); Settings::LogSettings(); Camera::RegisterFactory("image", std::make_unique()); @@ -472,7 +472,7 @@ void Java_org_citra_citra_1emu_NativeLibrary_ReloadSettings(JNIEnv* env, GameSettings::LoadOverrides(program_id); } - Settings::Apply(); + system.ApplySettings(); } jstring Java_org_citra_citra_1emu_NativeLibrary_GetUserSetting(JNIEnv* env, diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index 3a9640c6e8..8397992a26 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -343,7 +343,7 @@ int main(int argc, char** argv) { // Apply the command line arguments Settings::values.gdbstub_port = gdb_port; Settings::values.use_gdbstub = use_gdbstub; - Settings::Apply(); + system.ApplySettings(); // Register frontend applets Frontend::RegisterDefaultApplets(); diff --git a/src/citra_qt/configuration/configure_dialog.cpp b/src/citra_qt/configuration/configure_dialog.cpp index 1b00ddbe3f..254a636411 100644 --- a/src/citra_qt/configuration/configure_dialog.cpp +++ b/src/citra_qt/configuration/configure_dialog.cpp @@ -8,10 +8,13 @@ #include "citra_qt/configuration/configure_dialog.h" #include "citra_qt/hotkeys.h" #include "common/settings.h" +#include "core/core.h" #include "ui_configure.h" -ConfigureDialog::ConfigureDialog(QWidget* parent, HotkeyRegistry& registry, bool enable_web_config) - : QDialog(parent), ui(std::make_unique()), registry(registry) { +ConfigureDialog::ConfigureDialog(QWidget* parent, HotkeyRegistry& registry_, Core::System& system_, + bool enable_web_config) + : QDialog(parent), ui{std::make_unique()}, registry{registry_}, + system{system_} { Settings::SetConfiguringGlobal(true); ui->setupUi(this); @@ -68,7 +71,7 @@ void ConfigureDialog::ApplyConfiguration() { ui->webTab->ApplyConfiguration(); ui->uiTab->ApplyConfiguration(); ui->storageTab->ApplyConfiguration(); - Settings::Apply(); + system.ApplySettings(); Settings::LogSettings(); } diff --git a/src/citra_qt/configuration/configure_dialog.h b/src/citra_qt/configuration/configure_dialog.h index 3631ab20af..23b32a1974 100644 --- a/src/citra_qt/configuration/configure_dialog.h +++ b/src/citra_qt/configuration/configure_dialog.h @@ -13,11 +13,15 @@ namespace Ui { class ConfigureDialog; } +namespace Core { +class System; +} + class ConfigureDialog : public QDialog { Q_OBJECT public: - explicit ConfigureDialog(QWidget* parent, HotkeyRegistry& registry, + explicit ConfigureDialog(QWidget* parent, HotkeyRegistry& registry, Core::System& system, bool enable_web_config = true); ~ConfigureDialog() override; @@ -37,4 +41,5 @@ private: std::unique_ptr ui; HotkeyRegistry& registry; + Core::System& system; }; diff --git a/src/citra_qt/configuration/configure_per_game.cpp b/src/citra_qt/configuration/configure_per_game.cpp index fa11e3240d..a0163dfa78 100644 --- a/src/citra_qt/configuration/configure_per_game.cpp +++ b/src/citra_qt/configuration/configure_per_game.cpp @@ -102,7 +102,7 @@ void ConfigurePerGame::ApplyConfiguration() { audio_tab->ApplyConfiguration(); debug_tab->ApplyConfiguration(); - Settings::Apply(); + system.ApplySettings(); Settings::LogSettings(); game_config->Save(); diff --git a/src/citra_qt/dumping/dumping_dialog.cpp b/src/citra_qt/dumping/dumping_dialog.cpp index bbe69f2454..cd05d87d9f 100644 --- a/src/citra_qt/dumping/dumping_dialog.cpp +++ b/src/citra_qt/dumping/dumping_dialog.cpp @@ -8,10 +8,11 @@ #include "citra_qt/dumping/options_dialog.h" #include "citra_qt/uisettings.h" #include "common/settings.h" +#include "core/core.h" #include "ui_dumping_dialog.h" -DumpingDialog::DumpingDialog(QWidget* parent) - : QDialog(parent), ui(std::make_unique()) { +DumpingDialog::DumpingDialog(QWidget* parent, Core::System& system_) + : QDialog(parent), ui{std::make_unique()}, system{system_} { ui->setupUi(this); @@ -216,5 +217,5 @@ void DumpingDialog::ApplyConfiguration() { Settings::values.audio_encoder_options = ui->audioEncoderOptionsLineEdit->text().toStdString(); Settings::values.audio_bitrate = ui->audioBitrateSpinBox->value(); UISettings::values.video_dumping_path = last_path; - Settings::Apply(); + system.ApplySettings(); } diff --git a/src/citra_qt/dumping/dumping_dialog.h b/src/citra_qt/dumping/dumping_dialog.h index 284f215c31..f0153f1796 100644 --- a/src/citra_qt/dumping/dumping_dialog.h +++ b/src/citra_qt/dumping/dumping_dialog.h @@ -10,13 +10,17 @@ namespace Ui { class DumpingDialog; } +namespace Core { +class System; +} + class QLineEdit; class DumpingDialog : public QDialog { Q_OBJECT public: - explicit DumpingDialog(QWidget* parent); + explicit DumpingDialog(QWidget* parent, Core::System& system); ~DumpingDialog() override; QString GetFilePath() const; @@ -32,6 +36,7 @@ private: QLineEdit* line_edit); std::unique_ptr ui; + Core::System& system; QString last_path; diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 7a16bbfd36..46b8fdfb2f 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -140,7 +140,7 @@ void GMainWindow::ShowTelemetryCallout() { "

Would you like to share your usage data with us?"); if (QMessageBox::question(this, tr("Telemetry"), telemetry_message) == QMessageBox::Yes) { NetSettings::values.enable_telemetry = true; - Settings::Apply(); + system.ApplySettings(); } } @@ -1140,7 +1140,7 @@ void GMainWindow::BootGame(const QString& filename) { const std::string config_file_name = title_id == 0 ? name : fmt::format("{:016X}", title_id); Config per_game_config(config_file_name, Config::ConfigType::PerGameConfig); - Settings::Apply(); + system.ApplySettings(); LOG_INFO(Frontend, "Using per game config file for title id {}", config_file_name); Settings::LogSettings(); @@ -1883,7 +1883,7 @@ void GMainWindow::ChangeScreenLayout() { } Settings::values.layout_option = new_layout; - Settings::Apply(); + system.ApplySettings(); UpdateSecondaryWindowVisibility(); } @@ -1911,18 +1911,18 @@ void GMainWindow::ToggleScreenLayout() { Settings::values.layout_option = new_layout; SyncMenuUISettings(); - Settings::Apply(); + system.ApplySettings(); UpdateSecondaryWindowVisibility(); } void GMainWindow::OnSwapScreens() { Settings::values.swap_screen = ui->action_Screen_Layout_Swap_Screens->isChecked(); - Settings::Apply(); + system.ApplySettings(); } void GMainWindow::OnRotateScreens() { Settings::values.upright_screen = ui->action_Screen_Layout_Upright_Screens->isChecked(); - Settings::Apply(); + system.ApplySettings(); } void GMainWindow::TriggerSwapScreens() { @@ -1961,7 +1961,7 @@ void GMainWindow::OnLoadState() { void GMainWindow::OnConfigure() { game_list->SetDirectoryWatcherEnabled(false); Settings::SetConfiguringGlobal(true); - ConfigureDialog configureDialog(this, hotkey_registry, + ConfigureDialog configureDialog(this, hotkey_registry, system, !multiplayer_state->IsHostingPublicRoom()); connect(&configureDialog, &ConfigureDialog::LanguageChanged, this, &GMainWindow::OnLanguageChanged); @@ -2278,7 +2278,7 @@ void GMainWindow::OnOpenFFmpeg() { #endif void GMainWindow::OnStartVideoDumping() { - DumpingDialog dialog(this); + DumpingDialog dialog(this, system); if (dialog.exec() != QDialog::DialogCode::Accepted) { ui->action_Dump_Video->setChecked(false); return; diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 4e3500a389..fc104ab451 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -176,5 +176,5 @@ if (CITRA_USE_PRECOMPILED_HEADERS) target_precompile_headers(citra_common PRIVATE precompiled_headers.h) endif() if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux" AND CMAKE_CXX_COMPILER_ID STREQUAL GNU) - target_link_libraries(citra_common PRIVATE backtrace) + target_link_libraries(citra_common PRIVATE backtrace dl) endif() diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 19a52263c6..9d76751f44 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -5,18 +5,8 @@ #include #include #include "audio_core/dsp_interface.h" +#include "common/file_util.h" #include "common/settings.h" -#include "core/core.h" -#include "core/gdbstub/gdbstub.h" -#include "core/hle/kernel/shared_page.h" -#include "core/hle/service/cam/cam.h" -#include "core/hle/service/hid/hid.h" -#include "core/hle/service/ir/ir_rst.h" -#include "core/hle/service/ir/ir_user.h" -#include "core/hle/service/mic_u.h" -#include "core/hle/service/plgldr/plgldr.h" -#include "video_core/renderer_base.h" -#include "video_core/video_core.h" namespace Settings { @@ -75,64 +65,6 @@ std::string_view GetTextureFilterName(TextureFilter filter) { Values values = {}; static bool configuring_global = true; -void Apply() { - GDBStub::SetServerPort(values.gdbstub_port.GetValue()); - GDBStub::ToggleServer(values.use_gdbstub.GetValue()); - - VideoCore::g_shader_jit_enabled = values.use_shader_jit.GetValue(); - VideoCore::g_hw_shader_enabled = values.use_hw_shader.GetValue(); - VideoCore::g_hw_shader_accurate_mul = values.shaders_accurate_mul.GetValue(); - -#ifndef ANDROID - if (VideoCore::g_renderer) { - VideoCore::g_renderer->UpdateCurrentFramebufferLayout(); - } -#endif - - if (VideoCore::g_renderer) { - auto& settings = VideoCore::g_renderer->Settings(); - settings.bg_color_update_requested = true; - settings.sampler_update_requested = true; - settings.shader_update_requested = true; - settings.texture_filter_update_requested = true; - } - - auto& system = Core::System::GetInstance(); - if (system.IsPoweredOn()) { - system.CoreTiming().UpdateClockSpeed(values.cpu_clock_percentage.GetValue()); - Core::DSP().SetSink(values.output_type.GetValue(), values.output_device.GetValue()); - Core::DSP().EnableStretching(values.enable_audio_stretching.GetValue()); - - auto hid = Service::HID::GetModule(system); - if (hid) { - hid->ReloadInputDevices(); - } - - auto apt = Service::APT::GetModule(system); - if (apt) { - apt->GetAppletManager()->ReloadInputDevices(); - } - - auto sm = system.ServiceManager(); - auto ir_user = sm.GetService("ir:USER"); - if (ir_user) - ir_user->ReloadInputDevices(); - auto ir_rst = sm.GetService("ir:rst"); - if (ir_rst) - ir_rst->ReloadInputDevices(); - - auto cam = Service::CAM::GetModule(system); - if (cam) { - cam->ReloadCameraDevices(); - } - - Service::MIC::ReloadMic(system); - } - - Service::PLGLDR::PLG_LDR::SetEnabled(values.plugin_loader_enabled.GetValue()); - Service::PLGLDR::PLG_LDR::SetAllowGameChangeState(values.allow_plugin_loader.GetValue()); -} - void LogSettings() { const auto log_setting = [](std::string_view name, const auto& value) { LOG_INFO(Config, "{}: {}", name, value); diff --git a/src/common/settings.h b/src/common/settings.h index 67c7b182c8..c7bf565fe3 100644 --- a/src/common/settings.h +++ b/src/common/settings.h @@ -525,7 +525,6 @@ void SetConfiguringGlobal(bool is_global); float Volume(); -void Apply(); void LogSettings(); // Restore the global state of all applicable settings in the Values struct diff --git a/src/core/core.cpp b/src/core/core.cpp index f3bbc12d73..4ac4d0fd04 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -13,9 +13,9 @@ #include "common/arch.h" #include "common/logging/log.h" #include "common/settings.h" -#include "common/texture.h" #include "core/arm/arm_interface.h" #include "core/arm/exclusive_monitor.h" +#include "core/hle/service/cam/cam.h" #if CITRA_ARCH(x86_64) || CITRA_ARCH(arm64) #include "core/arm/dynarmic/arm_dynarmic.h" #endif @@ -24,19 +24,22 @@ #include "core/core.h" #include "core/core_timing.h" #include "core/dumping/backend.h" -#include "core/dumping/ffmpeg_backend.h" #include "core/frontend/image_interface.h" #include "core/gdbstub/gdbstub.h" #include "core/global.h" -#include "core/hle/kernel/client_port.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/process.h" #include "core/hle/kernel/thread.h" #include "core/hle/service/apt/applet_manager.h" #include "core/hle/service/apt/apt.h" +#include "core/hle/service/cam/cam.h" #include "core/hle/service/fs/archive.h" #include "core/hle/service/gsp/gsp.h" -#include "core/hle/service/pm/pm_app.h" +#include "core/hle/service/hid/hid.h" +#include "core/hle/service/ir/ir_rst.h" +#include "core/hle/service/ir/ir_user.h" +#include "core/hle/service/mic_u.h" +#include "core/hle/service/plgldr/plgldr.h" #include "core/hle/service/service.h" #include "core/hle/service/sm/sm.h" #include "core/hw/gpu.h" @@ -597,6 +600,64 @@ void System::Reset() { } } +void System::ApplySettings() { + GDBStub::SetServerPort(Settings::values.gdbstub_port.GetValue()); + GDBStub::ToggleServer(Settings::values.use_gdbstub.GetValue()); + + VideoCore::g_shader_jit_enabled = Settings::values.use_shader_jit.GetValue(); + VideoCore::g_hw_shader_enabled = Settings::values.use_hw_shader.GetValue(); + VideoCore::g_hw_shader_accurate_mul = Settings::values.shaders_accurate_mul.GetValue(); + +#ifndef ANDROID + if (VideoCore::g_renderer) { + VideoCore::g_renderer->UpdateCurrentFramebufferLayout(); + } +#endif + + if (VideoCore::g_renderer) { + auto& settings = VideoCore::g_renderer->Settings(); + settings.bg_color_update_requested = true; + settings.sampler_update_requested = true; + settings.shader_update_requested = true; + settings.texture_filter_update_requested = true; + } + + if (IsPoweredOn()) { + CoreTiming().UpdateClockSpeed(Settings::values.cpu_clock_percentage.GetValue()); + Core::DSP().SetSink(Settings::values.output_type.GetValue(), + Settings::values.output_device.GetValue()); + Core::DSP().EnableStretching(Settings::values.enable_audio_stretching.GetValue()); + + auto hid = Service::HID::GetModule(*this); + if (hid) { + hid->ReloadInputDevices(); + } + + auto apt = Service::APT::GetModule(*this); + if (apt) { + apt->GetAppletManager()->ReloadInputDevices(); + } + + auto ir_user = service_manager->GetService("ir:USER"); + if (ir_user) + ir_user->ReloadInputDevices(); + auto ir_rst = service_manager->GetService("ir:rst"); + if (ir_rst) + ir_rst->ReloadInputDevices(); + + auto cam = Service::CAM::GetModule(*this); + if (cam) { + cam->ReloadCameraDevices(); + } + + Service::MIC::ReloadMic(*this); + } + + Service::PLGLDR::PLG_LDR::SetEnabled(Settings::values.plugin_loader_enabled.GetValue()); + Service::PLGLDR::PLG_LDR::SetAllowGameChangeState( + Settings::values.allow_plugin_loader.GetValue()); +} + template void System::serialize(Archive& ar, const unsigned int file_version) { diff --git a/src/core/core.h b/src/core/core.h index b29fc20e23..3b48912d1a 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -330,6 +330,9 @@ public: return false; } + /// Applies any changes to settings to this core instance. + void ApplySettings(); + private: /** * Initialize the emulated system. diff --git a/src/dedicated_room/CMakeLists.txt b/src/dedicated_room/CMakeLists.txt index 4df160a8d6..7f130a9a82 100644 --- a/src/dedicated_room/CMakeLists.txt +++ b/src/dedicated_room/CMakeLists.txt @@ -8,7 +8,7 @@ add_executable(citra-room create_target_directory_groups(citra-room) -target_link_libraries(citra-room PRIVATE citra_common citra_core network) +target_link_libraries(citra-room PRIVATE citra_common network) if (ENABLE_WEB_SERVICE) target_compile_definitions(citra-room PRIVATE -DENABLE_WEB_SERVICE) target_link_libraries(citra-room PRIVATE web_service) From d7b42603896ac152b4080c2482f908c2beb75e0a Mon Sep 17 00:00:00 2001 From: GPUCode Date: Mon, 3 Jul 2023 02:17:03 +0300 Subject: [PATCH 25/25] common: Address feedback --- src/android/app/src/main/jni/config.cpp | 8 +++++++- src/citra/config.cpp | 8 +++++++- src/citra_qt/main.cpp | 8 ++++---- src/common/logging/backend.cpp | 10 +++++----- src/common/logging/backend.h | 3 ++- src/common/logging/log_entry.h | 2 +- src/common/logging/types.h | 4 ++-- src/dedicated_room/citra-room.cpp | 2 +- 8 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/android/app/src/main/jni/config.cpp b/src/android/app/src/main/jni/config.cpp index 8c3273279a..7efdeefcde 100644 --- a/src/android/app/src/main/jni/config.cpp +++ b/src/android/app/src/main/jni/config.cpp @@ -7,8 +7,8 @@ #include #include #include - #include "common/file_util.h" +#include "common/logging/backend.h" #include "common/logging/log.h" #include "common/param_package.h" #include "common/settings.h" @@ -260,6 +260,12 @@ void Config::ReadValues() { // Miscellaneous ReadSetting("Miscellaneous", Settings::values.log_filter); + // Apply the log_filter setting as the logger has already been initialized + // and doesn't pick up the filter on its own. + Common::Log::Filter filter; + filter.ParseFilterString(Settings::values.log_filter.GetValue()); + Common::Log::SetGlobalFilter(filter); + // Debugging Settings::values.record_frame_times = sdl2_config->GetBoolean("Debugging", "record_frame_times", false); diff --git a/src/citra/config.cpp b/src/citra/config.cpp index 09fda6ca5a..5edaf42da5 100644 --- a/src/citra/config.cpp +++ b/src/citra/config.cpp @@ -11,8 +11,8 @@ #include "citra/config.h" #include "citra/default_ini.h" #include "common/file_util.h" +#include "common/logging/backend.h" #include "common/logging/log.h" -#include "common/param_package.h" #include "common/settings.h" #include "core/hle/service/service.h" #include "input_common/main.h" @@ -299,6 +299,12 @@ void Config::ReadValues() { // Miscellaneous ReadSetting("Miscellaneous", Settings::values.log_filter); + // Apply the log_filter setting as the logger has already been initialized + // and doesn't pick up the filter on its own. + Common::Log::Filter filter; + filter.ParseFilterString(Settings::values.log_filter.GetValue()); + Common::Log::SetGlobalFilter(filter); + // Debugging Settings::values.record_frame_times = sdl2_config->GetBoolean("Debugging", "record_frame_times", false); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 46b8fdfb2f..359abeea02 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -172,8 +172,9 @@ GMainWindow::GMainWindow(Core::System& system_) : ui{std::make_unique()}, system{system_}, movie{Core::Movie::GetInstance()}, config{std::make_unique()}, emu_thread{nullptr} { Common::Log::Initialize(); + Common::Log::Start(); + Debugger::ToggleConsole(); - Settings::LogSettings(); // register types to use in slots and signals qRegisterMetaType("std::size_t"); @@ -264,8 +265,6 @@ GMainWindow::GMainWindow(Core::System& system_) } #endif - Common::Log::Start(); - QStringList args = QApplication::arguments(); if (args.length() >= 2) { BootGame(args[1]); @@ -1143,9 +1142,10 @@ void GMainWindow::BootGame(const QString& filename) { system.ApplySettings(); LOG_INFO(Frontend, "Using per game config file for title id {}", config_file_name); - Settings::LogSettings(); } + Settings::LogSettings(); + // Save configurations UpdateUISettings(); game_list->SaveInterfaceLayout(); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index d868b26094..5c80cdff0f 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -202,7 +202,7 @@ public: return *instance; } - static void Initialize() { + static void Initialize(std::string_view log_file) { if (instance) { LOG_WARNING(Log, "Reinitializing logging backend"); return; @@ -212,8 +212,8 @@ public: void(FileUtil::CreateDir(log_dir)); Filter filter; filter.ParseFilterString(Settings::values.log_filter.GetValue()); - instance = std::unique_ptr(new Impl(log_dir + LOG_FILE, filter), - Deleter); + instance = std::unique_ptr( + new Impl(fmt::format("{}{}", log_dir, log_file), filter), Deleter); initialization_in_progress_suppress_logging = false; } @@ -414,8 +414,8 @@ private: }; } // namespace -void Initialize() { - Impl::Initialize(); +void Initialize(std::string_view log_file) { + Impl::Initialize(log_file.empty() ? LOG_FILE : log_file); } void Start() { diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index a0e80fe3c1..19a5ef0f46 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -4,6 +4,7 @@ #pragma once +#include #include "common/logging/filter.h" namespace Common::Log { @@ -11,7 +12,7 @@ namespace Common::Log { class Filter; /// Initializes the logging system. This should be the first thing called in main. -void Initialize(); +void Initialize(std::string_view log_file = ""); void Start(); diff --git a/src/common/logging/log_entry.h b/src/common/logging/log_entry.h index b28570071c..290ec784a5 100644 --- a/src/common/logging/log_entry.h +++ b/src/common/logging/log_entry.h @@ -19,7 +19,7 @@ struct Entry { Class log_class{}; Level log_level{}; const char* filename = nullptr; - unsigned int line_num = 0; + u32 line_num = 0; std::string function; std::string message; }; diff --git a/src/common/logging/types.h b/src/common/logging/types.h index e8fc3feade..f4db2dc424 100644 --- a/src/common/logging/types.h +++ b/src/common/logging/types.h @@ -20,7 +20,7 @@ enum class Level : u8 { Critical, ///< Major problems during execution that threaten the stability of the entire ///< application. - Count ///< Total number of logging levels + Count, ///< Total number of logging levels }; /** @@ -100,7 +100,7 @@ enum class Class : u8 { Movie, ///< Movie (Input Recording) Playback WebService, ///< Interface to Citra Web Services RPC_Server, ///< RPC server - Count ///< Total number of logging classes + Count, ///< Total number of logging classes }; } // namespace Common::Log diff --git a/src/dedicated_room/citra-room.cpp b/src/dedicated_room/citra-room.cpp index 78fef0e906..dab153dd99 100644 --- a/src/dedicated_room/citra-room.cpp +++ b/src/dedicated_room/citra-room.cpp @@ -150,7 +150,7 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin } static void InitializeLogging(const std::string& log_file) { - Common::Log::Initialize(); + Common::Log::Initialize(log_file); Common::Log::SetColorConsoleBackendEnabled(true); }