From b88c91dd3d03f1a452cdd48f0db4e010cb150753 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Tue, 12 May 2015 02:19:44 -0300 Subject: [PATCH] Common: Remove async logging It provided a large increase in complexity of the logging system while having a negligible performance impact: the usage patterns of the ring buffer meant that each log contended with the logging thread, causing it to effectively act as a synchronous extra buffering. Also removed some broken code related to filtering of subclasses which was broken since it was introduced. (Which means no one ever used that feature anyway, since, 8 months later, no one ever complained.) --- src/citra/citra.cpp | 6 -- src/citra_qt/main.cpp | 6 -- src/common/logging/backend.cpp | 56 +++------------- src/common/logging/backend.h | 92 ++++----------------------- src/common/logging/filter.cpp | 41 ++---------- src/common/logging/filter.h | 20 ++---- src/common/logging/log.h | 6 +- src/common/logging/text_formatter.cpp | 19 +----- src/common/logging/text_formatter.h | 8 --- 9 files changed, 32 insertions(+), 222 deletions(-) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index 1d7e7f270..ca93d5b91 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -20,14 +20,8 @@ /// Application entry point int main(int argc, char **argv) { - std::shared_ptr logger = Log::InitGlobalLogger(); Log::Filter log_filter(Log::Level::Debug); Log::SetFilter(&log_filter); - std::thread logging_thread(Log::TextLoggingLoop, logger); - SCOPE_EXIT({ - logger->Close(); - logging_thread.join(); - }); if (argc < 2) { LOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified"); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index f115c5b6a..bb7acf61b 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -350,14 +350,8 @@ void GMainWindow::closeEvent(QCloseEvent* event) int main(int argc, char* argv[]) { - std::shared_ptr logger = Log::InitGlobalLogger(); Log::Filter log_filter(Log::Level::Info); Log::SetFilter(&log_filter); - std::thread logging_thread(Log::TextLoggingLoop, logger); - SCOPE_EXIT({ - logger->Close(); - logging_thread.join(); - }); QApplication::setAttribute(Qt::AA_X11InitThreads); QApplication app(argc, argv); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 7d3534a43..bd2c6a153 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -3,17 +3,17 @@ // Refer to the license.txt file included. #include +#include +#include -#include "common/assert.h" - +#include "common/common_funcs.h" // snprintf compatibility define #include "common/logging/backend.h" +#include "common/logging/filter.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" namespace Log { -static std::shared_ptr global_logger; - /// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. #define ALL_LOG_CLASSES() \ CLS(Log) \ @@ -55,28 +55,8 @@ static std::shared_ptr global_logger; SUB(Render, OpenGL) \ CLS(Loader) -Logger::Logger() { - // Register logging classes so that they can be queried at runtime - size_t parent_class; - all_classes.reserve((size_t)Class::Count); - -#define CLS(x) \ - all_classes.push_back(Class::x); \ - parent_class = all_classes.size() - 1; -#define SUB(x, y) \ - all_classes.push_back(Class::x##_##y); \ - all_classes[parent_class].num_children += 1; - - ALL_LOG_CLASSES() -#undef CLS -#undef SUB - - // Ensures that ALL_LOG_CLASSES isn't missing any entries. - DEBUG_ASSERT(all_classes.size() == (size_t)Class::Count); -} - // GetClassName is a macro defined by Windows.h, grrr... -const char* Logger::GetLogClassName(Class log_class) { +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; @@ -87,7 +67,7 @@ const char* Logger::GetLogClassName(Class log_class) { return "Unknown"; } -const char* Logger::GetLevelName(Level log_level) { +const char* GetLevelName(Level log_level) { #define LVL(x) case Level::x: return #x switch (log_level) { LVL(Trace); @@ -101,19 +81,6 @@ const char* Logger::GetLevelName(Level log_level) { #undef LVL } -void Logger::LogMessage(Entry entry) { - ring_buffer.Push(std::move(entry)); -} - -size_t Logger::GetEntries(Entry* out_buffer, size_t buffer_len) { - return ring_buffer.BlockingPop(out_buffer, buffer_len); -} - -std::shared_ptr InitGlobalLogger() { - global_logger = std::make_shared(); - return global_logger; -} - Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, const char* function, const char* format, va_list args) { @@ -138,7 +105,7 @@ Entry CreateEntry(Class log_class, Level log_level, return std::move(entry); } -static Filter* filter; +static Filter* filter = nullptr; void SetFilter(Filter* new_filter) { filter = new_filter; @@ -147,7 +114,7 @@ void SetFilter(Filter* new_filter) { void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr, const char* function, const char* format, ...) { - if (!filter->CheckMessage(log_class, log_level)) + if (filter != nullptr && !filter->CheckMessage(log_class, log_level)) return; va_list args; @@ -156,12 +123,7 @@ void LogMessage(Class log_class, Level log_level, filename, line_nr, function, format, args); va_end(args); - if (global_logger != nullptr && !global_logger->IsClosed()) { - global_logger->LogMessage(std::move(entry)); - } else { - // Fall back to directly printing to stderr - PrintMessage(entry); - } + PrintColoredMessage(entry); } } diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 3114f864c..c1f4d08e4 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -4,17 +4,17 @@ #pragma once +#include #include -#include -#include +#include +#include -#include "common/concurrent_ring_buffer.h" - -#include "common/logging/filter.h" #include "common/logging/log.h" namespace Log { +class Filter; + /** * 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. @@ -48,89 +48,21 @@ struct Entry { } }; -struct ClassInfo { - Class log_class; - - /** - * Total number of (direct or indirect) sub classes this class has. If any, they follow in - * sequence after this class in the class list. - */ - unsigned int num_children = 0; - - ClassInfo(Class log_class) : log_class(log_class) {} -}; +/** + * 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); /** - * Logging management class. This class has the dual purpose of acting as an exchange point between - * the logging clients and the log outputter, as well as containing reflection info about available - * log classes. + * Returns the name of the passed log level as a C-string. */ -class Logger { -private: - using Buffer = Common::ConcurrentRingBuffer; - -public: - static const size_t QUEUE_CLOSED = Buffer::QUEUE_CLOSED; - - Logger(); - - /** - * Returns a list of all vector classes and subclasses. The sequence returned is a pre-order of - * classes and subclasses, which together with the `num_children` field in ClassInfo, allows - * you to recover the hierarchy. - */ - const std::vector& GetClasses() const { return all_classes; } - - /** - * Returns the name of the passed log class as a C-string. Subclasses are separated by periods - * instead of underscores as in the enumeration. - */ - static const char* GetLogClassName(Class log_class); - - /** - * Returns the name of the passed log level as a C-string. - */ - static const char* GetLevelName(Level log_level); - - /** - * Appends a messages to the log buffer. - * @note This function is thread safe. - */ - void LogMessage(Entry entry); - - /** - * Retrieves a batch of messages from the log buffer, blocking until they are available. - * @note This function is thread safe. - * - * @param out_buffer Destination buffer that will receive the log entries. - * @param buffer_len The maximum size of `out_buffer`. - * @return The number of entries stored. In case the logger is shutting down, `QUEUE_CLOSED` is - * returned, no entries are stored and the logger should shutdown. - */ - size_t GetEntries(Entry* out_buffer, size_t buffer_len); - - /** - * Initiates a shutdown of the logger. This will indicate to log output clients that they - * should shutdown. - */ - void Close() { ring_buffer.Close(); } - - /** - * Returns true if Close() has already been called on the Logger. - */ - bool IsClosed() const { return ring_buffer.IsClosed(); } - -private: - Buffer ring_buffer; - std::vector all_classes; -}; +const char* GetLevelName(Level log_level); /// Creates a log entry by formatting the given source location, and message. Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, const char* function, const char* format, va_list args); -/// Initializes the default Logger. -std::shared_ptr InitGlobalLogger(); void SetFilter(Filter* filter); diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index 50f2e13f4..55cc8888a 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -22,16 +22,6 @@ void Filter::SetClassLevel(Class log_class, Level level) { class_levels[static_cast(log_class)] = level; } -void Filter::SetSubclassesLevel(const ClassInfo& log_class, Level level) { - const size_t log_class_i = static_cast(log_class.log_class); - - const size_t begin = log_class_i + 1; - const size_t end = begin + log_class.num_children; - for (size_t i = begin; begin < end; ++i) { - class_levels[i] = level; - } -} - void Filter::ParseFilterString(const std::string& filter_str) { auto clause_begin = filter_str.cbegin(); while (clause_begin != filter_str.cend()) { @@ -53,7 +43,7 @@ void Filter::ParseFilterString(const std::string& filter_str) { template static Level GetLevelByName(const It begin, const It end) { for (u8 i = 0; i < static_cast(Level::Count); ++i) { - const char* level_name = Logger::GetLevelName(static_cast(i)); + const char* level_name = GetLevelName(static_cast(i)); if (Common::ComparePartialString(begin, end, level_name)) { return static_cast(i); } @@ -64,7 +54,7 @@ static Level GetLevelByName(const It begin, const It end) { template static Class GetClassByName(const It begin, const It end) { for (ClassType i = 0; i < static_cast(Class::Count); ++i) { - const char* level_name = Logger::GetLogClassName(static_cast(i)); + const char* level_name = GetLogClassName(static_cast(i)); if (Common::ComparePartialString(begin, end, level_name)) { return static_cast(i); } @@ -72,20 +62,6 @@ static Class GetClassByName(const It begin, const It end) { return Class::Count; } -template -static InputIt find_last(InputIt begin, const InputIt end, const T& value) { - auto match = end; - while (begin != end) { - auto new_match = std::find(begin, end, value); - if (new_match != end) { - match = new_match; - ++new_match; - } - begin = new_match; - } - return match; -} - bool Filter::ParseFilterRule(const std::string::const_iterator begin, const std::string::const_iterator end) { auto level_separator = std::find(begin, end, ':'); @@ -106,22 +82,13 @@ bool Filter::ParseFilterRule(const std::string::const_iterator begin, return true; } - auto class_name_end = find_last(begin, level_separator, '.'); - if (class_name_end != level_separator && - !Common::ComparePartialString(class_name_end + 1, level_separator, "*")) { - class_name_end = level_separator; - } - - const Class log_class = GetClassByName(begin, class_name_end); + const Class log_class = GetClassByName(begin, level_separator); if (log_class == Class::Count) { LOG_ERROR(Log, "Unknown log class in filter: %s", std::string(begin, end).c_str()); return false; } - if (class_name_end == level_separator) { - SetClassLevel(log_class, level); - } - SetSubclassesLevel(log_class, level); + SetClassLevel(log_class, level); return true; } diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index b53e4e633..0b71ea3b2 100644 --- a/src/common/logging/filter.h +++ b/src/common/logging/filter.h @@ -11,8 +11,6 @@ namespace Log { -struct ClassInfo; - /** * 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 @@ -27,29 +25,19 @@ public: 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); - /** - * Sets the minimum level of all of `log_class` subclasses to `level`. The level of `log_class` - * itself is not changed. - */ - void SetSubclassesLevel(const ClassInfo& 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. - * A rule for a given class also affects all of its subclasses. `*` wildcards are allowed and - * can be used to apply a rule to all classes or to all subclasses of a class without affecting - * the parent class. `` 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. + * `*` 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 and all subclasses (Service.FS, Service.APT, - * etc.) to Info. - * - `Service.*:Debug` -- Sets the level of all Service subclasses to Debug, while leaving the - * level of Service unchanged. + * - `Service:Info` -- Sets the level of Service to Info. * - `Service.FS:Trace` -- Sets the level of the Service.FS class to Trace. */ void ParseFilterString(const std::string& filter_str); diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 123641cb4..fd87ddbe6 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -78,11 +78,7 @@ enum class Class : ClassType { Count ///< Total number of logging classes }; -/** - * Logs a message to the global logger. This proxy exists to avoid exposing the details of the - * Logger class, including the ConcurrentRingBuffer template, to all files that desire to log - * messages, reducing unecessary recompilations. - */ +/// Logs a message to the global logger. void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr, const char* function, #ifdef _MSC_VER diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index 45be6d0a1..94f3dfc1f 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -46,8 +46,8 @@ void FormatLogMessage(const Entry& entry, char* out_text, size_t text_len) { unsigned int time_seconds = static_cast(entry.timestamp.count() / 1000000); unsigned int time_fractional = static_cast(entry.timestamp.count() % 1000000); - const char* class_name = Logger::GetLogClassName(entry.log_class); - const char* level_name = Logger::GetLevelName(entry.log_level); + const char* class_name = GetLogClassName(entry.log_class); + const char* level_name = GetLevelName(entry.log_level); snprintf(out_text, text_len, "[%4u.%06u] %s <%s> %s: %s", time_seconds, time_fractional, class_name, level_name, @@ -116,19 +116,4 @@ void PrintColoredMessage(const Entry& entry) { #endif } -void TextLoggingLoop(std::shared_ptr logger) { - std::array entry_buffer; - - while (true) { - size_t num_entries = logger->GetEntries(entry_buffer.data(), entry_buffer.size()); - if (num_entries == Logger::QUEUE_CLOSED) { - break; - } - for (size_t i = 0; i < num_entries; ++i) { - const Entry& entry = entry_buffer[i]; - PrintColoredMessage(entry); - } - } -} - } diff --git a/src/common/logging/text_formatter.h b/src/common/logging/text_formatter.h index 8474a1904..5b82f043f 100644 --- a/src/common/logging/text_formatter.h +++ b/src/common/logging/text_formatter.h @@ -5,11 +5,9 @@ #pragma once #include -#include namespace Log { -class Logger; struct Entry; /** @@ -31,10 +29,4 @@ void PrintMessage(const Entry& entry); /// Prints the same message as `PrintMessage`, but colored acoording to the severity level. void PrintColoredMessage(const Entry& entry); -/** - * Logging loop that repeatedly reads messages from the provided logger and prints them to the - * console. It is the baseline barebones log outputter. - */ -void TextLoggingLoop(std::shared_ptr logger); - }