From 95d8085fa42ca73773a06f2bd5622f69aef0e598 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 May 2021 15:21:51 -0400 Subject: SL-10297: Make LLSingletonBase::llerrs() et al. runtime variadic. Instead of accepting a fixed list of (const char* p1="", etc.), accept (std::initializer_list). Accepting a std::initializer_list in your parameter list allows coding (e.g.) func({T0, T1, T2, ... }); -- in other words, you can pass the initializer_list a brace-enclosed list of an arbitrary number of instances of T. Using std::string_view instead of const char* means we can pass *either* const char* or std::string. string_view is cheaply constructed from either, allowing uniform treatment within the function. Constructing string_view from std::string simply extracts the pointer and length from the std::string. Constructing string_view from const char* (e.g. a "string literal") requires scanning the string for its terminating nul byte -- but that would be necessary even if the scan were deferred until the function body. Since string_view stores the length, the scan still happens only once. --- indra/llcommon/llsingleton.h | 81 +++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 42 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 30a5b21cf8..b9570d42db 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -34,6 +34,7 @@ #include "lockstatic.h" #include "llthread.h" // on_main_thread() #include "llmainthreadtask.h" +#include class LLSingletonBase: private boost::noncopyable { @@ -111,14 +112,10 @@ protected: void capture_dependency(); // delegate logging calls to llsingleton.cpp - static void logerrs(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void logwarns(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void loginfos(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void logdebugs(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); + static void logerrs (std::initializer_list); + static void logwarns (std::initializer_list); + static void loginfos (std::initializer_list); + static void logdebugs(std::initializer_list); static std::string demangle(const char* mangled); // these classname() declarations restate template functions declared in // llerror.h because we avoid #including that here @@ -327,8 +324,8 @@ private: // init stack to its previous size BEFORE logging so log-machinery // LLSingletons don't record a dependency on DERIVED_TYPE! LLSingleton_manage_master().reset_initializing(prev_size); - logwarns("Error constructing ", classname().c_str(), - ": ", err.what()); + logwarns({"Error constructing ", classname(), + ": ", err.what()}); // There isn't a separate EInitState value meaning "we attempted // to construct this LLSingleton subclass but could not," so use // DELETED. That seems slightly more appropriate than UNINITIALIZED. @@ -356,8 +353,8 @@ private: // BEFORE logging, so log-machinery LLSingletons don't record a // dependency on DERIVED_TYPE! pop_initializing(lk->mInstance); - logwarns("Error in ", classname().c_str(), - "::initSingleton(): ", err.what()); + logwarns({"Error in ", classname(), + "::initSingleton(): ", err.what()}); // Get rid of the instance entirely. This call depends on our // recursive_mutex. We could have a deleteSingleton(LockStatic&) // overload and pass lk, but we don't strictly need it. @@ -506,9 +503,9 @@ public: case CONSTRUCTING: // here if DERIVED_TYPE's constructor (directly or indirectly) // calls DERIVED_TYPE::getInstance() - logerrs("Tried to access singleton ", - classname().c_str(), - " from singleton constructor!"); + logerrs({"Tried to access singleton ", + classname(), + " from singleton constructor!"}); return nullptr; case INITIALIZING: @@ -523,9 +520,9 @@ public: case DELETED: // called after deleteSingleton() - logwarns("Trying to access deleted singleton ", - classname().c_str(), - " -- creating new instance"); + logwarns({"Trying to access deleted singleton ", + classname(), + " -- creating new instance"}); // fall through case UNINITIALIZED: case QUEUED: @@ -552,8 +549,8 @@ public: } // unlock 'lk' // Per the comment block above, dispatch to the main thread. - loginfos(classname().c_str(), - "::getInstance() dispatching to main thread"); + loginfos({classname(), + "::getInstance() dispatching to main thread"}); auto instance = LLMainThreadTask::dispatch( [](){ // VERY IMPORTANT to call getInstance() on the main thread, @@ -563,16 +560,16 @@ public: // the main thread processes them, only the FIRST such request // actually constructs the instance -- every subsequent one // simply returns the existing instance. - loginfos(classname().c_str(), - "::getInstance() on main thread"); + loginfos({classname(), + "::getInstance() on main thread"}); return getInstance(); }); // record the dependency chain tracked on THIS thread, not the main // thread (consider a getInstance() overload with a tag param that // suppresses dep tracking when dispatched to the main thread) capture_dependency(instance); - loginfos(classname().c_str(), - "::getInstance() returning on requesting thread"); + loginfos({classname(), + "::getInstance() returning on requesting thread"}); return instance; } @@ -641,16 +638,16 @@ private: // For organizational purposes this function shouldn't be called twice if (lk->mInitState != super::UNINITIALIZED) { - super::logerrs("Tried to initialize singleton ", - super::template classname().c_str(), - " twice!"); + super::logerrs({"Tried to initialize singleton ", + super::template classname(), + " twice!"}); return nullptr; } else if (on_main_thread()) { // on the main thread, simply construct instance while holding lock - super::logdebugs(super::template classname().c_str(), - "::initParamSingleton()"); + super::logdebugs({super::template classname(), + "::initParamSingleton()"}); super::constructSingleton(lk, std::forward(args)...); return lk->mInstance; } @@ -662,8 +659,8 @@ private: lk->mInitState = super::QUEUED; // very important to unlock here so main thread can actually process lk.unlock(); - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() dispatching to main thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() dispatching to main thread"}); // Normally it would be the height of folly to reference-bind // 'args' into a lambda to be executed on some other thread! By // the time that thread executed the lambda, the references would @@ -674,12 +671,12 @@ private: // references. auto instance = LLMainThreadTask::dispatch( [&](){ - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() on main thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() on main thread"}); return initParamSingleton_(std::forward(args)...); }); - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() returning on requesting thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() returning on requesting thread"}); return instance; } } @@ -707,14 +704,14 @@ public: { case super::UNINITIALIZED: case super::QUEUED: - super::logerrs("Uninitialized param singleton ", - super::template classname().c_str()); + super::logerrs({"Uninitialized param singleton ", + super::template classname()}); break; case super::CONSTRUCTING: - super::logerrs("Tried to access param singleton ", - super::template classname().c_str(), - " from singleton constructor!"); + super::logerrs({"Tried to access param singleton ", + super::template classname(), + " from singleton constructor!"}); break; case super::INITIALIZING: @@ -726,8 +723,8 @@ public: return lk->mInstance; case super::DELETED: - super::logerrs("Trying to access deleted param singleton ", - super::template classname().c_str()); + super::logerrs({"Trying to access deleted param singleton ", + super::template classname()}); break; } -- cgit v1.2.3 From 147c66d67ce83778fc3f102dd132ec095e6032fc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 09:46:49 -0400 Subject: SL-10297: Simplify implementation of LLSingletonBase::logwarns() etc. Introduce 'string_params' typedef for std::initialization_list, and make logwarns(), loginfos(), logdebugs() and logerrs() accept const string_params&. Eliminate the central log() function in llsingleton.cpp that used LL_VLOGS(). To cache the result of a (moderately expensive) Log::shouldLog() call, LL_VLOGS() wants its CallSite object to be static -- but of course the shouldLog() result will differ for different ELevel values, so LL_VLOGS() instantiates a static array of CallSite instances. It seems silly to funnel distinct logwarns(), etc., functions through a common log() function only to have LL_VLOGS() tease apart ELevel values again. Instead, make logwarns() directly invoke LL_WARNS(), and similarly for the rest. To reduce boilerplate in these distinct functions, teach std::ostream how to stream a string_params instance by looping over its elements. Then each logwarns(), etc., function can simply stream its string_params argument to LL_WARNS() or whichever. In particular, eliminate the LLERROR_CRASH macro in logerrs(). The fact that it invokes LL_ERRS() ensures that its LL_ENDL macro will crash the viewer. --- indra/llcommon/llsingleton.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index b9570d42db..2eb39c6c8c 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -112,10 +112,13 @@ protected: void capture_dependency(); // delegate logging calls to llsingleton.cpp - static void logerrs (std::initializer_list); - static void logwarns (std::initializer_list); - static void loginfos (std::initializer_list); - static void logdebugs(std::initializer_list); +public: + typedef std::initializer_list string_params; +protected: + static void logerrs (const string_params&); + static void logwarns (const string_params&); + static void loginfos (const string_params&); + static void logdebugs(const string_params&); static std::string demangle(const char* mangled); // these classname() declarations restate template functions declared in // llerror.h because we avoid #including that here -- cgit v1.2.3 From d10badf0d23f48665239117838c5daf0fd667e01 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 16:26:53 -0400 Subject: SL-10297: #include in llsingleton.h --- indra/llcommon/llsingleton.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 2eb39c6c8c..163c08099f 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -27,14 +27,15 @@ #include #include +#include #include -#include +#include #include +#include #include "mutex.h" #include "lockstatic.h" #include "llthread.h" // on_main_thread() #include "llmainthreadtask.h" -#include class LLSingletonBase: private boost::noncopyable { -- cgit v1.2.3 From 24501dfa0ee3fd6f5755deb1bc5261cd297a2bc7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 22:35:09 -0400 Subject: SL-10297: Use initializer_list vs. . This is somewhat more expensive for string literals, but switching to std::string_view implies more extensive changes, to be considered separately. --- indra/llcommon/llsingleton.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 163c08099f..7c81d65a8b 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include "mutex.h" @@ -114,7 +113,7 @@ protected: // delegate logging calls to llsingleton.cpp public: - typedef std::initializer_list string_params; + typedef std::initializer_list string_params; protected: static void logerrs (const string_params&); static void logwarns (const string_params&); -- cgit v1.2.3