From 6ebf94f0e4b19874353c0d412ff744f849b86a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Tue, 5 Mar 2024 09:39:38 +0100 Subject: [PATCH 1/2] Withdrawn changes from #2858 as they break ABI compat --- srtcore/api.cpp | 10 ++++---- srtcore/logging.cpp | 4 +++- srtcore/logging.h | 57 +++++++++++++++++++++++++++++++++++---------- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index f9ac6a8d1..e77469de2 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -4656,21 +4656,21 @@ void setloglevel(LogLevel::type ll) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.max_level = ll; - srt_logger_config.updateLoggersState(); +// srt_logger_config.updateLoggersState(); } void addlogfa(LogFA fa) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.enabled_fa.set(fa, true); - srt_logger_config.updateLoggersState(); +// srt_logger_config.updateLoggersState(); } void dellogfa(LogFA fa) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.enabled_fa.set(fa, false); - srt_logger_config.updateLoggersState(); +// srt_logger_config.updateLoggersState(); } void resetlogfa(set fas) @@ -4678,7 +4678,7 @@ void resetlogfa(set fas) ScopedLock gg(srt_logger_config.mutex); for (int i = 0; i <= SRT_LOGFA_LASTNONE; ++i) srt_logger_config.enabled_fa.set(i, fas.count(i)); - srt_logger_config.updateLoggersState(); +// srt_logger_config.updateLoggersState(); } void resetlogfa(const int* fara, size_t fara_size) @@ -4687,7 +4687,7 @@ void resetlogfa(const int* fara, size_t fara_size) srt_logger_config.enabled_fa.reset(); for (const int* i = fara; i != fara + fara_size; ++i) srt_logger_config.enabled_fa.set(*i, true); - srt_logger_config.updateLoggersState(); +// srt_logger_config.updateLoggersState(); } void setlogstream(std::ostream& stream) diff --git a/srtcore/logging.cpp b/srtcore/logging.cpp index d309b1b8a..c1625916d 100644 --- a/srtcore/logging.cpp +++ b/srtcore/logging.cpp @@ -23,6 +23,8 @@ using namespace std; namespace srt_logging { +/* Blocked temporarily until 1.6.0. This causes ABI incompat with 1.5.0 line. + // Note: subscribe() and unsubscribe() functions are being called // in the global constructor and destructor only, as the // Logger objects (and inside them also their LogDispatcher) @@ -85,7 +87,7 @@ void LogDispatcher::SendLogLine(const char* file, int line, const std::string& a } src_config->unlock(); } - +*/ #if ENABLE_LOGGING diff --git a/srtcore/logging.h b/srtcore/logging.h index c17781c24..88a2bbfa3 100644 --- a/srtcore/logging.h +++ b/srtcore/logging.h @@ -20,7 +20,7 @@ written by #include #include #include -#include +//#include #include #include #ifdef _WIN32 @@ -116,7 +116,7 @@ struct LogConfig void* loghandler_opaque; srt::sync::Mutex mutex; int flags; - std::vector loggers; + // std::vector loggers; LogConfig(const fa_bitset_t& efa, LogLevel::type l = LogLevel::warning, @@ -139,10 +139,11 @@ struct LogConfig SRT_ATTR_RELEASE(mutex) void unlock() { mutex.unlock(); } - +/* void subscribe(LogDispatcher*); void unsubscribe(LogDispatcher*); void updateLoggersState(); + */ }; // The LogDispatcher class represents the object that is responsible for @@ -154,7 +155,7 @@ struct SRT_API LogDispatcher LogLevel::type level; static const size_t MAX_PREFIX_SIZE = 32; char prefix[MAX_PREFIX_SIZE+1]; - srt::sync::atomic enabled; +// srt::sync::atomic enabled; LogConfig* src_config; bool isset(int flg) { return (src_config->flags & flg) != 0; } @@ -165,7 +166,7 @@ struct SRT_API LogDispatcher const char* logger_pfx /*[[nullable]]*/, LogConfig& config): fa(functional_area), level(log_level), - enabled(false), +// enabled(false), src_config(&config) { // XXX stpcpy desired, but not enough portable @@ -193,18 +194,17 @@ struct SRT_API LogDispatcher prefix[MAX_PREFIX_SIZE] = '\0'; #endif } - config.subscribe(this); - Update(); +// config.subscribe(this); +// Update(); } ~LogDispatcher() { - src_config->unsubscribe(this); +// src_config->unsubscribe(this); } - void Update(); - - bool CheckEnabled() { return enabled; } + // void Update(); + bool CheckEnabled(); // { return enabled; } void CreateLogLinePrefix(std::ostringstream&); void SendLogLine(const char* file, int line, const std::string& area, const std::string& sl); @@ -428,6 +428,22 @@ class Logger } }; +inline bool LogDispatcher::CheckEnabled() +{ + // Don't use enabler caching. Check enabled state every time. + + // These assume to be atomically read, so the lock is not needed + // (note that writing to this field is still mutex-protected). + // It's also no problem if the level was changed at the moment + // when the enabler check is tested here. Worst case, the log + // will be printed just a moment after it was turned off. + const LogConfig* config = src_config; // to enforce using const operator[] + int configured_enabled_fa = config->enabled_fa[fa]; + int configured_maxlevel = config->max_level; + + return configured_enabled_fa && level <= configured_maxlevel; +} + #if HAVE_CXX11 @@ -478,7 +494,24 @@ inline void LogDispatcher::PrintLogLine(const char* file SRT_ATR_UNUSED, int lin #endif // HAVE_CXX11 +// SendLogLine can be compiled normally. It's intermediately used by: +// - Proxy object, which is replaced by DummyProxy when !ENABLE_LOGGING +// - PrintLogLine, which has empty body when !ENABLE_LOGGING +inline void LogDispatcher::SendLogLine(const char* file, int line, const std::string& area, const std::string& msg) +{ + src_config->lock(); + if ( src_config->loghandler_fn ) + { + (*src_config->loghandler_fn)(src_config->loghandler_opaque, int(level), file, line, area.c_str(), msg.c_str()); + } + else if ( src_config->log_stream ) + { + (*src_config->log_stream) << msg; + (*src_config->log_stream).flush(); + } + src_config->unlock(); +} + } #endif // INC_SRT_LOGGING_H - From 3601861c13ff8da2d1b7e4d7ec9a5e04978a9f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Ma=C5=82ecki?= Date: Tue, 5 Mar 2024 10:12:33 +0100 Subject: [PATCH 2/2] [core] Restored logger perf improvements that were blocked up to 1.6.0 --- srtcore/api.cpp | 10 ++++---- srtcore/logging.cpp | 4 +--- srtcore/logging.h | 56 +++++++++------------------------------------ 3 files changed, 17 insertions(+), 53 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index e77469de2..f9ac6a8d1 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -4656,21 +4656,21 @@ void setloglevel(LogLevel::type ll) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.max_level = ll; -// srt_logger_config.updateLoggersState(); + srt_logger_config.updateLoggersState(); } void addlogfa(LogFA fa) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.enabled_fa.set(fa, true); -// srt_logger_config.updateLoggersState(); + srt_logger_config.updateLoggersState(); } void dellogfa(LogFA fa) { ScopedLock gg(srt_logger_config.mutex); srt_logger_config.enabled_fa.set(fa, false); -// srt_logger_config.updateLoggersState(); + srt_logger_config.updateLoggersState(); } void resetlogfa(set fas) @@ -4678,7 +4678,7 @@ void resetlogfa(set fas) ScopedLock gg(srt_logger_config.mutex); for (int i = 0; i <= SRT_LOGFA_LASTNONE; ++i) srt_logger_config.enabled_fa.set(i, fas.count(i)); -// srt_logger_config.updateLoggersState(); + srt_logger_config.updateLoggersState(); } void resetlogfa(const int* fara, size_t fara_size) @@ -4687,7 +4687,7 @@ void resetlogfa(const int* fara, size_t fara_size) srt_logger_config.enabled_fa.reset(); for (const int* i = fara; i != fara + fara_size; ++i) srt_logger_config.enabled_fa.set(*i, true); -// srt_logger_config.updateLoggersState(); + srt_logger_config.updateLoggersState(); } void setlogstream(std::ostream& stream) diff --git a/srtcore/logging.cpp b/srtcore/logging.cpp index c1625916d..d309b1b8a 100644 --- a/srtcore/logging.cpp +++ b/srtcore/logging.cpp @@ -23,8 +23,6 @@ using namespace std; namespace srt_logging { -/* Blocked temporarily until 1.6.0. This causes ABI incompat with 1.5.0 line. - // Note: subscribe() and unsubscribe() functions are being called // in the global constructor and destructor only, as the // Logger objects (and inside them also their LogDispatcher) @@ -87,7 +85,7 @@ void LogDispatcher::SendLogLine(const char* file, int line, const std::string& a } src_config->unlock(); } -*/ + #if ENABLE_LOGGING diff --git a/srtcore/logging.h b/srtcore/logging.h index 88a2bbfa3..2d599bb49 100644 --- a/srtcore/logging.h +++ b/srtcore/logging.h @@ -20,7 +20,7 @@ written by #include #include #include -//#include +#include #include #include #ifdef _WIN32 @@ -116,7 +116,7 @@ struct LogConfig void* loghandler_opaque; srt::sync::Mutex mutex; int flags; - // std::vector loggers; + std::vector loggers; LogConfig(const fa_bitset_t& efa, LogLevel::type l = LogLevel::warning, @@ -139,11 +139,10 @@ struct LogConfig SRT_ATTR_RELEASE(mutex) void unlock() { mutex.unlock(); } -/* + void subscribe(LogDispatcher*); void unsubscribe(LogDispatcher*); void updateLoggersState(); - */ }; // The LogDispatcher class represents the object that is responsible for @@ -155,7 +154,7 @@ struct SRT_API LogDispatcher LogLevel::type level; static const size_t MAX_PREFIX_SIZE = 32; char prefix[MAX_PREFIX_SIZE+1]; -// srt::sync::atomic enabled; + srt::sync::atomic enabled; LogConfig* src_config; bool isset(int flg) { return (src_config->flags & flg) != 0; } @@ -166,7 +165,7 @@ struct SRT_API LogDispatcher const char* logger_pfx /*[[nullable]]*/, LogConfig& config): fa(functional_area), level(log_level), -// enabled(false), + enabled(false), src_config(&config) { // XXX stpcpy desired, but not enough portable @@ -194,17 +193,18 @@ struct SRT_API LogDispatcher prefix[MAX_PREFIX_SIZE] = '\0'; #endif } -// config.subscribe(this); -// Update(); + config.subscribe(this); + Update(); } ~LogDispatcher() { -// src_config->unsubscribe(this); + src_config->unsubscribe(this); } - // void Update(); - bool CheckEnabled(); // { return enabled; } + void Update(); + + bool CheckEnabled() { return enabled; } void CreateLogLinePrefix(std::ostringstream&); void SendLogLine(const char* file, int line, const std::string& area, const std::string& sl); @@ -428,22 +428,6 @@ class Logger } }; -inline bool LogDispatcher::CheckEnabled() -{ - // Don't use enabler caching. Check enabled state every time. - - // These assume to be atomically read, so the lock is not needed - // (note that writing to this field is still mutex-protected). - // It's also no problem if the level was changed at the moment - // when the enabler check is tested here. Worst case, the log - // will be printed just a moment after it was turned off. - const LogConfig* config = src_config; // to enforce using const operator[] - int configured_enabled_fa = config->enabled_fa[fa]; - int configured_maxlevel = config->max_level; - - return configured_enabled_fa && level <= configured_maxlevel; -} - #if HAVE_CXX11 @@ -494,24 +478,6 @@ inline void LogDispatcher::PrintLogLine(const char* file SRT_ATR_UNUSED, int lin #endif // HAVE_CXX11 -// SendLogLine can be compiled normally. It's intermediately used by: -// - Proxy object, which is replaced by DummyProxy when !ENABLE_LOGGING -// - PrintLogLine, which has empty body when !ENABLE_LOGGING -inline void LogDispatcher::SendLogLine(const char* file, int line, const std::string& area, const std::string& msg) -{ - src_config->lock(); - if ( src_config->loghandler_fn ) - { - (*src_config->loghandler_fn)(src_config->loghandler_opaque, int(level), file, line, area.c_str(), msg.c_str()); - } - else if ( src_config->log_stream ) - { - (*src_config->log_stream) << msg; - (*src_config->log_stream).flush(); - } - src_config->unlock(); -} - } #endif // INC_SRT_LOGGING_H