Introduce tracy instrumentation of mutex in LLSingleton, LLInstanceTracker and logging

5 files changed, 69 insertions, 49 deletions

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 6c3b9c9542..41c69ba194 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -55,6 +55,7 @@
 #include "llsingleton.h"
 #include "llstl.h"
 #include "lltimer.h"
+#include "llprofiler.h"
 
 // On Mac, got:
 // #error "Boost.Stacktrace requires `_Unwind_Backtrace` function. Define
@@ -506,7 +507,7 @@ namespace
         LLError::TimeFunction               mTimeFunction;
 
         Recorders                           mRecorders;
-        LLCoros::RMutex                     mRecorderMutex;
+        LL_PROFILE_MUTEX_NAMED(LLCoros::RMutex, mRecorderMutex, "Log Recorders");
 
         int                                 mShouldLogCallCounter;
 
@@ -529,7 +530,6 @@ namespace
         mCrashFunction(NULL),
         mTimeFunction(NULL),
         mRecorders(),
-        mRecorderMutex(),
         mShouldLogCallCounter(0)
     {
     }
@@ -1044,7 +1044,7 @@ namespace LLError
             return;
         }
         SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
-        std::unique_lock lock(s->mRecorderMutex);
+        std::unique_lock lock(s->mRecorderMutex); LL_PROFILE_MUTEX_LOCK(s->mRecorderMutex);
         s->mRecorders.push_back(recorder);
     }
 
@@ -1055,7 +1055,7 @@ namespace LLError
             return;
         }
         SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
-        std::unique_lock lock(s->mRecorderMutex);
+        std::unique_lock lock(s->mRecorderMutex); LL_PROFILE_MUTEX_LOCK(s->mRecorderMutex);
         s->mRecorders.erase(std::remove(s->mRecorders.begin(), s->mRecorders.end(), recorder),
                             s->mRecorders.end());
     }
@@ -1104,7 +1104,7 @@ namespace LLError
     std::shared_ptr<RECORDER> findRecorder()
     {
         SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
-        std::unique_lock lock(s->mRecorderMutex);
+        std::unique_lock lock(s->mRecorderMutex); LL_PROFILE_MUTEX_LOCK(s->mRecorderMutex);
         return findRecorderPos<RECORDER>(s).first;
     }
 
@@ -1115,7 +1115,7 @@ namespace LLError
     bool removeRecorder()
     {
         SettingsConfigPtr s = Globals::getInstance()->getSettingsConfig();
-        std::unique_lock lock(s->mRecorderMutex);
+        std::unique_lock lock(s->mRecorderMutex); LL_PROFILE_MUTEX_LOCK(s->mRecorderMutex);
         auto found = findRecorderPos<RECORDER>(s);
         if (found.first)
         {
@@ -1221,7 +1221,7 @@ namespace
 
         std::string escaped_message;
 
-        std::unique_lock lock(s->mRecorderMutex);
+        std::unique_lock lock(s->mRecorderMutex); LL_PROFILE_MUTEX_LOCK(s->mRecorderMutex);
         for (LLError::RecorderPtr& r : s->mRecorders)
         {
             if (!r->enabled())
@@ -1280,24 +1280,21 @@ namespace
 }
 
 namespace {
-    // We need a couple different mutexes, but we want to use the same mechanism
-    // for both. Make getMutex() a template function with different instances
-    // for different MutexDiscriminator values.
-    enum MutexDiscriminator
-    {
-        LOG_MUTEX,
-        STACKS_MUTEX
-    };
     // Some logging calls happen very early in processing -- so early that our
     // module-static variables aren't yet initialized. getMutex() wraps a
     // function-static LLMutex so that early calls can still have a valid
     // LLMutex instance.
-    template <MutexDiscriminator MTX>
-    LLMutex* getMutex()
+    auto getLogMutex()
+    {
+        // guaranteed to be initialized the first time control reaches here
+        static LL_PROFILE_MUTEX_NAMED(std::recursive_mutex, sLogMutex, "Log Mutex");
+        return &sLogMutex;
+    }
+    auto getStacksMutex()
     {
         // guaranteed to be initialized the first time control reaches here
-        static LLMutex sMutex;
-        return &sMutex;
+        static LL_PROFILE_MUTEX_NAMED(std::recursive_mutex, sStacksMutex, "Stacks Mutex");
+        return &sStacksMutex;
     }
 
     bool checkLevelMap(const LevelMap& map, const std::string& key,
@@ -1347,8 +1344,8 @@ namespace LLError
     bool Log::shouldLog(CallSite& site)
     {
         LL_PROFILE_ZONE_SCOPED_CATEGORY_LOGGING;
-        LLMutexTrylock lock(getMutex<LOG_MUTEX>(), 5);
-        if (!lock.isLocked())
+        std::unique_lock lock(*getLogMutex(), std::try_to_lock); LL_PROFILE_MUTEX_LOCK(*getLogMutex());
+        if (!lock)
         {
             return false;
         }
@@ -1392,8 +1389,8 @@ namespace LLError
     void Log::flush(const std::ostringstream& out, const CallSite& site)
     {
         LL_PROFILE_ZONE_SCOPED_CATEGORY_LOGGING;
-        LLMutexTrylock lock(getMutex<LOG_MUTEX>(),5);
-        if (!lock.isLocked())
+        std::unique_lock lock(*getLogMutex(), std::try_to_lock); LL_PROFILE_MUTEX_LOCK(*getLogMutex());
+        if (!lock)
         {
             return;
         }
@@ -1523,8 +1520,8 @@ namespace LLError
     //static
     void LLCallStacks::push(const char* function, const int line)
     {
-        LLMutexTrylock lock(getMutex<STACKS_MUTEX>(), 5);
-        if (!lock.isLocked())
+        std::unique_lock lock(*getStacksMutex(), std::try_to_lock); LL_PROFILE_MUTEX_LOCK(*getStacksMutex());
+        if (!lock)
         {
             return;
         }
@@ -1548,8 +1545,8 @@ namespace LLError
     //static
     void LLCallStacks::end(const std::ostringstream& out)
     {
-        LLMutexTrylock lock(getMutex<STACKS_MUTEX>(), 5);
-        if (!lock.isLocked())
+        std::unique_lock lock(*getStacksMutex(), std::try_to_lock); LL_PROFILE_MUTEX_LOCK(*getStacksMutex());
+        if (!lock)
         {
             return;
         }
@@ -1565,8 +1562,8 @@ namespace LLError
     //static
     void LLCallStacks::print()
     {
-        LLMutexTrylock lock(getMutex<STACKS_MUTEX>(), 5);
-        if (!lock.isLocked())
+        std::unique_lock lock(*getStacksMutex(), std::try_to_lock); LL_PROFILE_MUTEX_LOCK(*getStacksMutex());
+        if (!lock)
         {
             return;
         }
diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h
index 3232a0e219..92b26354a1 100644
--- a/indra/llcommon/llinstancetracker.h
+++ b/indra/llcommon/llinstancetracker.h
@@ -52,7 +52,7 @@ namespace LLInstanceTrackerPrivate
     struct StaticBase
     {
         // We need to be able to lock static data while manipulating it.
-        std::mutex mMutex;
+        LL_PROFILE_MUTEX_NAMED(std::mutex, mMutex, "InstanceTracker Data");
     };
 
     void logerrs(const char* cls, const std::string&, const std::string&, const std::string&);
@@ -101,7 +101,8 @@ public:
 
     static size_t instanceCount()
     {
-        return LockStatic()->mMap.size();
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
+        return lock->mMap.size();
     }
 
     // snapshot of std::pair<const KEY, std::shared_ptr<SUBCLASS>> pairs, for
@@ -236,7 +237,7 @@ public:
 
     static ptr_t getInstance(const KEY& k)
     {
-        LockStatic lock;
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
         const InstanceMap& map(lock->mMap);
         typename InstanceMap::const_iterator found = map.find(k);
         return (found == map.end()) ? NULL : found->second;
@@ -252,19 +253,19 @@ protected:
         ptr_t ptr(static_cast<T*>(this), [](T*){});
         // save corresponding weak_ptr for future reference
         mSelf = ptr;
-        LockStatic lock;
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
         add_(lock, key, ptr);
     }
 public:
     virtual ~LLInstanceTracker()
     {
-        LockStatic lock;
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
         remove_(lock);
     }
 protected:
     virtual void setKey(KEY key)
     {
-        LockStatic lock;
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
         // Even though the shared_ptr we store in our map has a no-op deleter
         // for T itself, letting the use count decrement to 0 will still
         // delete the use-count object. Capture the shared_ptr we just removed
@@ -376,7 +377,8 @@ public:
 
     static size_t instanceCount()
     {
-        return LockStatic()->mSet.size();
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
+        return lock->mSet.size();
     }
 
     // snapshot of std::shared_ptr<SUBCLASS> pointers
@@ -488,14 +490,16 @@ protected:
         // save corresponding weak_ptr for future reference
         mSelf = ptr;
         // Also store it in our class-static set to track this instance.
-        LockStatic()->mSet.emplace(ptr);
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
+        lock->mSet.emplace(ptr);
     }
 public:
     virtual ~LLInstanceTracker()
     {
         // convert weak_ptr to shared_ptr because that's what we store in our
         // InstanceSet
-        LockStatic()->mSet.erase(mSelf.lock());
+        LockStatic lock; LL_PROFILE_MUTEX_LOCK(lock->mMutex);
+        lock->mSet.erase(mSelf.lock());
     }
 protected:
     LLInstanceTracker(const LLInstanceTracker& other):
diff --git a/indra/llcommon/llprofiler.h b/indra/llcommon/llprofiler.h
index b7d0522336..f6a4d24747 100644
--- a/indra/llcommon/llprofiler.h
+++ b/indra/llcommon/llprofiler.h
@@ -102,6 +102,12 @@ extern thread_local bool gProfilerEnabled;
         #define LL_PROFILE_ZONE_ERR(name)               LL_PROFILE_ZONE_NAMED_COLOR( name, 0XFF0000  )  // RGB yellow
         #define LL_PROFILE_ZONE_INFO(name)              LL_PROFILE_ZONE_NAMED_COLOR( name, 0X00FFFF  )  // RGB cyan
         #define LL_PROFILE_ZONE_WARN(name)              LL_PROFILE_ZONE_NAMED_COLOR( name, 0x0FFFF00 )  // RGB red
+
+        #define LL_PROFILE_MUTEX(type, varname)                     TracyLockable(type, varname)
+        #define LL_PROFILE_MUTEX_NAMED(type, varname, desc)         TracyLockableN(type, varname, desc)
+        #define LL_PROFILE_MUTEX_SHARED(type, varname)              TracySharedLockable(type, varname)
+        #define LL_PROFILE_MUTEX_SHARED_NAMED(type, varname, desc)  TracySharedLockableN(type, varname, desc)
+        #define LL_PROFILE_MUTEX_LOCK(varname) { auto& mutex = varname; LockMark(mutex); }
     #endif
     #if LL_PROFILER_CONFIGURATION == LL_PROFILER_CONFIG_FAST_TIMER
         #define LL_PROFILER_FRAME_END
@@ -118,6 +124,12 @@ extern thread_local bool gProfilerEnabled;
         #define LL_PROFILE_ZONE_ERR(name)               (void)(name); // Not supported
         #define LL_PROFILE_ZONE_INFO(name)              (void)(name); // Not supported
         #define LL_PROFILE_ZONE_WARN(name)              (void)(name); // Not supported
+
+        #define LL_PROFILE_MUTEX(type, varname) type varname
+        #define LL_PROFILE_MUTEX_NAMED(type, varname, desc) type varname
+        #define LL_PROFILE_MUTEX_SHARED(type, varname) type varname
+        #define LL_PROFILE_MUTEX_SHARED_NAMED(type, varname, desc) type varname
+        #define LL_PROFILE_MUTEX_LOCK(varname) // LL_PROFILE_MUTEX_LOCK is a no-op when Tracy is disabled
     #endif
     #if LL_PROFILER_CONFIGURATION == LL_PROFILER_CONFIG_TRACY_FAST_TIMER
         #define LL_PROFILER_FRAME_END                   FrameMark
@@ -133,6 +145,12 @@ extern thread_local bool gProfilerEnabled;
         #define LL_PROFILE_ZONE_ERR(name)               LL_PROFILE_ZONE_NAMED_COLOR( name, 0XFF0000  )  // RGB yellow
         #define LL_PROFILE_ZONE_INFO(name)              LL_PROFILE_ZONE_NAMED_COLOR( name, 0X00FFFF  )  // RGB cyan
         #define LL_PROFILE_ZONE_WARN(name)              LL_PROFILE_ZONE_NAMED_COLOR( name, 0x0FFFF00 )  // RGB red
+
+        #define LL_PROFILE_MUTEX(type, varname)                     TracyLockable(type, varname)
+        #define LL_PROFILE_MUTEX_NAMED(type, varname, desc)         TracyLockableN(type, varname, desc)
+        #define LL_PROFILE_MUTEX_SHARED(type, varname)              TracySharedLockable(type, varname)
+        #define LL_PROFILE_MUTEX_SHARED_NAMED(type, varname, desc)  TracySharedLockableN(type, varname, desc)
+        #define LL_PROFILE_MUTEX_LOCK(varname) { auto& mutex = varname; LockMark(mutex); } // see https://github.com/wolfpld/tracy/issues/575
     #endif
 #else
     #define LL_PROFILER_FRAME_END
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index d00e703a10..05dc3cde79 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -59,9 +59,8 @@ private:
     // it's safe to log -- which involves querying a different LLSingleton --
     // which requires accessing the master list.
     typedef std::recursive_mutex mutex_t;
-    typedef std::unique_lock<mutex_t> lock_t;
-
-    mutex_t mMutex;
+    LL_PROFILE_MUTEX_NAMED(mutex_t, mMutex, "Singleton MasterList");
+    typedef std::unique_lock<decltype(mMutex)> lock_t;
 
 public:
     // Instantiate this to both obtain a reference to MasterList::instance()
diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index 316831cd74..b5659e053c 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -35,6 +35,8 @@
 #include "lockstatic.h"
 #include "llthread.h"               // on_main_thread()
 #include "llmainthreadtask.h"
+#include "llprofiler.h"
+#include "llerror.h"
 
 #ifdef LL_WINDOWS
 #pragma warning(push)
@@ -298,7 +300,7 @@ private:
         // Use a recursive_mutex in case of constructor circularity. With a
         // non-recursive mutex, that would result in deadlock.
         typedef std::recursive_mutex mutex_t;
-        mutex_t mMutex;             // LockStatic looks for mMutex
+        LL_PROFILE_MUTEX_NAMED(mutex_t, mMutex, "Singleton Data"); // LockStatic looks for mMutex
 
         EInitState      mInitState{UNINITIALIZED};
         DERIVED_TYPE*   mInstance{nullptr};
@@ -420,7 +422,7 @@ protected:
         // deleteSingleton() to defend against manual deletion. When we moved
         // cleanup to deleteSingleton(), we hit crashes due to dangling
         // pointers in the MasterList.
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
         lk->mInstance  = nullptr;
         lk->mInitState = DELETED;
 
@@ -448,7 +450,7 @@ public:
         // Hold the lock while we call cleanupSingleton() and the destructor.
         // Our destructor also instantiates LockStatic, requiring a recursive
         // mutex.
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
         // of course, only cleanup and delete if there's something there
         if (lk->mInstance)
         {
@@ -505,7 +507,7 @@ public:
         { // nested scope for 'lk'
             // In case racing threads call getInstance() at the same moment,
             // serialize the calls.
-            LockStatic lk;
+            LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
 
             switch (lk->mInitState)
             {
@@ -595,7 +597,7 @@ public:
     static bool instanceExists()
     {
         // defend any access to sData from racing threads
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
         return lk->mInitState == INITIALIZED;
     }
 
@@ -605,7 +607,7 @@ public:
     static bool wasDeleted()
     {
         // defend any access to sData from racing threads
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
         return lk->mInitState == DELETED;
     }
 };
@@ -644,7 +646,7 @@ private:
         // In case racing threads both call initParamSingleton() at the same
         // time, serialize them. One should initialize; the other should see
         // mInitState already set.
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
         // For organizational purposes this function shouldn't be called twice
         if (lk->mInitState != super::UNINITIALIZED)
         {
@@ -708,7 +710,7 @@ public:
     {
         // In case racing threads call getInstance() at the same moment as
         // initParamSingleton(), serialize the calls.
-        LockStatic lk;
+        LockStatic lk; LL_PROFILE_MUTEX_LOCK(lk->mMutex);
 
         switch (lk->mInitState)
         {