From f7af56d7b6a72b6c0029f0fc6b9c4f65a133ed4a Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 28 Sep 2025 18:18:28 -0500 Subject: [PATCH 1/4] Simplify Logger::Prefix (function that adds prefix) --- src/common/Log.cpp | 15 ++++++++------- src/common/Log.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/common/Log.cpp b/src/common/Log.cpp index c7271df2c6..1cbfd0cde0 100644 --- a/src/common/Log.cpp +++ b/src/common/Log.cpp @@ -45,15 +45,16 @@ namespace Log { Logger::Logger(Str::StringRef name, std::string prefix, Level defaultLevel) : filterLevel(new Cvar::Cvar( "logs.level." + name, "Log::Level - logs from '" + name + "' below the level specified are filtered", 0, defaultLevel)), - prefix(prefix), enableSuppression(true) { + enableSuppression(true) + { + if (!prefix.empty()) { + // TODO allow prefixes without a space, e.g. a color code + this->prefix = prefix + " "; + } } - std::string Logger::Prefix(std::string message) const { - if (prefix.empty()) { - return message; - } else { - return prefix + " " + message; - } + std::string Logger::Prefix(Str::StringRef message) const { + return prefix + message; } void Logger::Dispatch(std::string message, Log::Level level, Str::StringRef format) { diff --git a/src/common/Log.h b/src/common/Log.h index 5046d80923..d6c684da66 100644 --- a/src/common/Log.h +++ b/src/common/Log.h @@ -118,7 +118,7 @@ namespace Log { private: void Dispatch(std::string message, Log::Level level, Str::StringRef format); - std::string Prefix(std::string message) const; + std::string Prefix(Str::StringRef message) const; // the cvar logs.level. std::shared_ptr> filterLevel; From 649dc2a2e2638bc076066ef54ac43dfde3f276b0 Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 28 Sep 2025 18:22:17 -0500 Subject: [PATCH 2/4] Fix unused Log::(Warn|Notice|Verbose|Debug) decls --- src/common/Log.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/common/Log.h b/src/common/Log.h index d6c684da66..0bb9578c23 100644 --- a/src/common/Log.h +++ b/src/common/Log.h @@ -131,22 +131,26 @@ namespace Log { /* * When debugging a function or before a logger is introduced for - * a module the following functions can be used for less typing. + * a module, the following signatures can be used for less typing. * However it shouldn't stay in production code because it * cannot be filtered and will clutter the console. + * + * These are not the real function declarations because macros are involved to get __LINE__ etc. */ +#if 0 template - void Warn( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ); + void Warn( Str::StringRef format, Args&& ... args ); template - void Notice( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ); + void Notice( Str::StringRef format, Args&& ... args ); template - void Verbose( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ); + void Verbose( Str::StringRef format, Args&& ... args ); template - void Debug( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ); + void Debug( Str::StringRef format, Args&& ... args ); +#endif /* * For messages which are not true log messages, but rather are produced by From 7f5dcd4ba88cf5b8dc2a5a4df10d4deef507d9f2 Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 28 Sep 2025 18:41:12 -0500 Subject: [PATCH 3/4] Access log suppression cvars less thread-unsafely In the engine, access the logs.suppression.* cvars through their objects, instead of looking them up in the cvar map via their names. Looking them up via name is seriously unsafe on a non-main thread. Simply reading the int or bool is still technically thread-unsafe, but should be fine in practice. The gamelogic still has to look up cvars by name, but there is no issue because logging is prohibited on non-main threads. Fixes, for practical purposes, https://github.com/DaemonEngine/Daemon-vulkan/issues/21. --- src/common/Log.cpp | 42 +++++++++++++++++++++--------- src/engine/framework/LogSystem.cpp | 10 ------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/common/Log.cpp b/src/common/Log.cpp index 1cbfd0cde0..e03e399667 100644 --- a/src/common/Log.cpp +++ b/src/common/Log.cpp @@ -30,6 +30,31 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "Common.h" +#ifdef BUILD_ENGINE +// TODO: when cvar multiple registration is available, just declare the cvars in each module. +static Cvar::Cvar suppressionEnabled( + "logs.suppression.enabled", "Whether to suppress log messages that are printed too many times", Cvar::NONE, true); +static Cvar::Range> suppressionInterval( + "logs.suppression.interval", "Interval in milliseconds for detecting log spam", Cvar::NONE, 2000, 1, 1000000); +static Cvar::Range> suppressionCount( + "logs.suppression.count", "Number of occurrences for a message to be considered log spam", Cvar::NONE, 10, 1, 1000000); +static Cvar::Range> suppressionBufSize( + "logs.suppression.bufferSize", "How many distinct messages to track for log suppression", Cvar::NONE, 50, 1, 1000000); + +#define GET_LOG_CVAR(type, name, object) (object.Get()) +#else +#define GET_LOG_CVAR(type, name, object) (GetCvarOrDie(name)) +template +static T GetCvarOrDie(Str::StringRef cvar) { + T value; + std::string valueString = Cvar::GetValue(cvar); + if (!Cvar::ParseCvarValue(valueString, value)) { + Sys::Error("Failed to deserialize cvar %s with value: %s", cvar, valueString); + } + return value; +} +#endif + namespace Log { Cvar::Cvar logExtendAll( "logs.writeSrcLocation.all", "Always print source code location for logs", Cvar::NONE, false ); @@ -137,15 +162,6 @@ namespace Log { } } - template - static T GetCvarOrDie(Str::StringRef cvar) { - T value; - std::string valueString = Cvar::GetValue(cvar); - if (!Cvar::ParseCvarValue(valueString, value)) { - Sys::Error("Failed to deserialize cvar %s with value: %s", cvar, valueString); - } - return value; - } namespace { // Log-spam suppression: if more than MAX_OCCURRENCES log messages with the same format string // are sent in less than INTERVAL_MS milliseconds, they will stop being printed. @@ -167,9 +183,9 @@ namespace Log { }; Result UpdateAndEvaluate(Str::StringRef messageFormat) { std::lock_guard lock(mutex); - int intervalMs = GetCvarOrDie("logs.suppression.interval"); - int maxOccurrences = GetCvarOrDie("logs.suppression.count"); - int bufferSize = GetCvarOrDie("logs.suppression.bufferSize"); + int intervalMs = GET_LOG_CVAR(int, "logs.suppression.interval", suppressionInterval); + int maxOccurrences = GET_LOG_CVAR(int, "logs.suppression.count", suppressionCount); + int bufferSize = GET_LOG_CVAR(int, "logs.suppression.bufferSize", suppressionBufSize); buf.resize(bufferSize); MessageStatistics* oldest = &buf[0]; int now = Sys::Milliseconds(); @@ -200,7 +216,7 @@ namespace Log { void DispatchWithSuppression(std::string message, Log::Level level, Str::StringRef format) { static LogSpamSuppressor suppressor; - if (level == Level::DEBUG || !GetCvarOrDie("logs.suppression.enabled")) { + if (level == Level::DEBUG || !GET_LOG_CVAR(bool, "logs.suppression.enabled", suppressionEnabled)) { DispatchByLevel(std::move(message), level); return; } diff --git a/src/engine/framework/LogSystem.cpp b/src/engine/framework/LogSystem.cpp index 1348189394..8c626c98ed 100644 --- a/src/engine/framework/LogSystem.cpp +++ b/src/engine/framework/LogSystem.cpp @@ -34,16 +34,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "LogSystem.h" namespace Log { - - static Cvar::Cvar suppressionEnabled( - "logs.suppression.enabled", "Whether to suppress log messages that are printed too many times", Cvar::NONE, true); - static Cvar::Range> suppressionInterval( - "logs.suppression.interval", "Interval in milliseconds for detecting log spam", Cvar::NONE, 2000, 1, 1000000); - static Cvar::Range> suppressionCount( - "logs.suppression.count", "Number of occurrences for a message to be considered log spam", Cvar::NONE, 10, 1, 1000000); - static Cvar::Range> suppressionBufSize( - "logs.suppression.bufferSize", "How many distinct messages to track for log suppression", Cvar::NONE, 50, 1, 1000000); - static Target* targets[MAX_TARGET_ID]; From a235cb3699d6e1249ce87b69be129bf4911329b2 Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 28 Sep 2025 19:53:30 -0500 Subject: [PATCH 4/4] Fix file:line logging in gamelogic logs.writeSrcLocation.* cvars no longer emit warnings due to duplicate registration, and are obeyed by the gamelogic. --- src/common/Log.cpp | 42 +++++++++++++++++++++++++++---------- src/common/Log.h | 52 +++++++++------------------------------------- 2 files changed, 41 insertions(+), 53 deletions(-) diff --git a/src/common/Log.cpp b/src/common/Log.cpp index e03e399667..dca3bb04ab 100644 --- a/src/common/Log.cpp +++ b/src/common/Log.cpp @@ -40,6 +40,16 @@ static Cvar::Range> suppressionCount( "logs.suppression.count", "Number of occurrences for a message to be considered log spam", Cvar::NONE, 10, 1, 1000000); static Cvar::Range> suppressionBufSize( "logs.suppression.bufferSize", "How many distinct messages to track for log suppression", Cvar::NONE, 50, 1, 1000000); +static Cvar::Cvar logExtendAll( + "logs.writeSrcLocation.all", "Always print source code location for logs", Cvar::NONE, false); +static Cvar::Cvar logExtendWarn( + "logs.writeSrcLocation.warn", "Print source code location for Warn logs", Cvar::NONE, false); +static Cvar::Cvar logExtendNotice( + "logs.writeSrcLocation.notice", "Print source code location for Notice logs", Cvar::NONE, false); +static Cvar::Cvar logExtendVerbose( + "logs.writeSrcLocation.verbose", "Print source code location for Verbose logs", Cvar::NONE, false); +static Cvar::Cvar logExtendDebug( + "logs.writeSrcLocation.debug", "Print source code location for Debug logs", Cvar::NONE, false); #define GET_LOG_CVAR(type, name, object) (object.Get()) #else @@ -56,16 +66,6 @@ static T GetCvarOrDie(Str::StringRef cvar) { #endif namespace Log { - Cvar::Cvar logExtendAll( - "logs.writeSrcLocation.all", "Always print source code location for logs", Cvar::NONE, false ); - Cvar::Cvar logExtendWarn( - "logs.writeSrcLocation.warn", "Print source code location for Warn logs", Cvar::NONE, false ); - Cvar::Cvar logExtendNotice( - "logs.writeSrcLocation.notice", "Print source code location for Notice logs", Cvar::NONE, false ); - Cvar::Cvar logExtendVerbose( - "logs.writeSrcLocation.verbose", "Print source code location for Verbose logs", Cvar::NONE, false ); - Cvar::Cvar logExtendDebug( - "logs.writeSrcLocation.debug", "Print source code location for Debug logs", Cvar::NONE, false ); Logger::Logger(Str::StringRef name, std::string prefix, Level defaultLevel) : filterLevel(new Cvar::Cvar( @@ -82,7 +82,27 @@ namespace Log { return prefix + message; } - void Logger::Dispatch(std::string message, Log::Level level, Str::StringRef format) { + static bool WantLocationInfo(Log::Level level) { + if (GET_LOG_CVAR(bool, "logs.writeSrcLocation.all", logExtendAll)) { + return true; + } + switch (level) { + case Log::Level::DEBUG: + return GET_LOG_CVAR(bool, "logs.writeSrcLocation.debug", logExtendDebug); + case Log::Level::VERBOSE: + return GET_LOG_CVAR(bool, "logs.writeSrcLocation.verbose", logExtendVerbose); + case Log::Level::NOTICE: + return GET_LOG_CVAR(bool, "logs.writeSrcLocation.notice", logExtendNotice); + case Log::Level::WARNING: + return GET_LOG_CVAR(bool, "logs.writeSrcLocation.warn", logExtendWarn); + } + ASSERT_UNREACHABLE(); + } + + void Logger::Dispatch(std::string message, Log::Level level, Str::StringRef format, const char* file, const char* function, int line) { + if (WantLocationInfo(level)) { + message = Str::Format("%s ^F(%s:%u, %s)", message, file, line, function); + } if (enableSuppression) { Log::DispatchWithSuppression(std::move(message), level, format); } else { diff --git a/src/common/Log.h b/src/common/Log.h index 0bb9578c23..dade1b62f8 100644 --- a/src/common/Log.h +++ b/src/common/Log.h @@ -51,12 +51,6 @@ namespace Log { // The default filtering level const Level DEFAULT_FILTER_LEVEL = Level::WARNING; - extern Cvar::Cvar logExtendAll; - extern Cvar::Cvar logExtendWarn; - extern Cvar::Cvar logExtendNotice; - extern Cvar::Cvar logExtendVerbose; - extern Cvar::Cvar logExtendDebug; - /* * Loggers are used to group logs by subsystems and allow logs * to be filtered by log level by subsystem. They are used like so @@ -116,7 +110,8 @@ namespace Log { Logger WithoutSuppression(); private: - void Dispatch(std::string message, Log::Level level, Str::StringRef format); + void Dispatch(std::string message, Log::Level level, Str::StringRef format, + const char* file, const char* function, int line); std::string Prefix(Str::StringRef message) const; @@ -203,62 +198,35 @@ namespace Log { // Logger - inline std::string AddSrcLocation( const std::string& message, - const char* file, const char* function, const int line, - const bool extend ) { - if ( logExtendAll.Get() || extend ) { - return message + Str::Format( " ^F(%s:%u, %s)", - file, line, function ); - } - - return message; - } - template void Logger::WarnExt( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ) { if ( filterLevel->Get() <= Level::WARNING ) { - this->Dispatch( - AddSrcLocation( - Prefix( Str::Format( format, std::forward( args ) ... ) ), - file, function, line, logExtendWarn.Get() - ), - Level::WARNING, format ); + this->Dispatch(Prefix(Str::Format(format, std::forward(args)...)), + Level::WARNING, format, file, function, line); } } template void Logger::NoticeExt( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ) { if ( filterLevel->Get() <= Level::NOTICE ) { - this->Dispatch( - AddSrcLocation( - Prefix( Str::Format( format, std::forward( args ) ... ) ), - file, function, line, logExtendNotice.Get() - ), - Level::NOTICE, format ); + this->Dispatch(Prefix(Str::Format(format, std::forward(args)...)), + Level::NOTICE, format, file, function, line); } } template void Logger::VerboseExt( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ) { if ( filterLevel->Get() <= Level::VERBOSE ) { - this->Dispatch( - AddSrcLocation( - Prefix( Str::Format( format, std::forward( args ) ... ) ), - file, function, line, logExtendVerbose.Get() - ), - Level::VERBOSE, format ); + this->Dispatch(Prefix(Str::Format(format, std::forward(args)...)), + Level::VERBOSE, format, file, function, line); } } template void Logger::DebugExt( const char* file, const char* function, const int line, Str::StringRef format, Args&& ... args ) { if ( filterLevel->Get() <= Level::DEBUG ) { - this->Dispatch( - AddSrcLocation( - Prefix( Str::Format( format, std::forward( args ) ... ) ), - file, function, line, logExtendDebug.Get() - ), - Level::DEBUG, format ); + this->Dispatch(Prefix(Str::Format(format, std::forward(args)...)), + Level::DEBUG, format, file, function, line); } }