From 05a3203d8274a0a0999faad128f629614b8d62c5 Mon Sep 17 00:00:00 2001
From: Richard Linden <none@none>
Date: Wed, 26 Sep 2012 17:04:57 -0700
Subject: [PATCH] SH-3275 WIP Run viewer metrics for object update messages
fixed various issues related to unit tests and LLThreadLocalPtr
initialization and teardown
---
indra/llcommon/llapr.cpp | 45 ++++++++++++++-------
indra/llcommon/llapr.h | 27 ++++++-------
indra/llcommon/llcommon.cpp | 6 +--
indra/llcommon/llthread.cpp | 5 ++-
indra/llcommon/llthread.h | 2 +-
indra/llcommon/lltrace.cpp | 33 ++++++++++-----
indra/llcommon/lltrace.h | 47 +++++++++++++---------
indra/llimage/tests/llimageworker_test.cpp | 5 +++
indra/test/test.cpp | 15 +++----
9 files changed, 108 insertions(+), 77 deletions(-)
diff --git a/indra/llcommon/llapr.cpp b/indra/llcommon/llapr.cpp
index 76749f8a911..e9930c10f70 100644
--- a/indra/llcommon/llapr.cpp
+++ b/indra/llcommon/llapr.cpp
@@ -52,7 +52,7 @@ void ll_init_apr()
if(!LLAPRFile::sAPRFilePoolp)
{
- LLAPRFile::sAPRFilePoolp = new LLVolatileAPRPool(FALSE) ;
+ LLAPRFile::sAPRFilePoolp = new LLVolatileAPRPool(FALSE);
}
LLThreadLocalPtrBase::initAllThreadLocalStorage();
@@ -79,6 +79,9 @@ void ll_cleanup_apr()
apr_thread_mutex_destroy(gCallStacksLogMutexp);
gCallStacksLogMutexp = NULL;
}
+
+ LLThreadLocalPtrBase::destroyAllThreadLocalStorage();
+
if (gAPRPoolp)
{
apr_pool_destroy(gAPRPoolp);
@@ -86,7 +89,7 @@ void ll_cleanup_apr()
}
if (LLAPRFile::sAPRFilePoolp)
{
- delete LLAPRFile::sAPRFilePoolp ;
+ delete LLAPRFile::sAPRFilePoolp ;
LLAPRFile::sAPRFilePoolp = NULL ;
}
apr_terminate();
@@ -483,9 +486,8 @@ S32 LLAPRFile::seek(apr_seek_where_t where, S32 offset)
//
bool LLThreadLocalPtrBase::sInitialized = false;
-LLThreadLocalPtrBase::LLThreadLocalPtrBase(void (*cleanup_func)(void*))
-: mCleanupFunc(cleanup_func),
- mThreadKey(NULL)
+LLThreadLocalPtrBase::LLThreadLocalPtrBase()
+: mThreadKey(NULL)
{
if (sInitialized)
{
@@ -494,8 +496,7 @@ LLThreadLocalPtrBase::LLThreadLocalPtrBase(void (*cleanup_func)(void*))
}
LLThreadLocalPtrBase::LLThreadLocalPtrBase( const LLThreadLocalPtrBase& other)
-: mCleanupFunc(other.mCleanupFunc),
- mThreadKey(NULL)
+: mThreadKey(NULL)
{
if (sInitialized)
{
@@ -522,7 +523,7 @@ void LLThreadLocalPtrBase::set( void* value )
void LLThreadLocalPtrBase::initStorage( )
{
- apr_status_t result = apr_threadkey_private_create(&mThreadKey, mCleanupFunc, gAPRPoolp);
+ apr_status_t result = apr_threadkey_private_create(&mThreadKey, NULL, gAPRPoolp);
if (result != APR_SUCCESS)
{
ll_apr_warn_status(result);
@@ -532,13 +533,16 @@ void LLThreadLocalPtrBase::initStorage( )
void LLThreadLocalPtrBase::destroyStorage()
{
- if (mThreadKey)
+ if (sInitialized)
{
- apr_status_t result = apr_threadkey_private_delete(mThreadKey);
- if (result != APR_SUCCESS)
+ if (mThreadKey)
{
- ll_apr_warn_status(result);
- llerrs << "Failed to delete thread local data" << llendl;
+ apr_status_t result = apr_threadkey_private_delete(mThreadKey);
+ if (result != APR_SUCCESS)
+ {
+ ll_apr_warn_status(result);
+ llerrs << "Failed to delete thread local data" << llendl;
+ }
}
}
}
@@ -547,16 +551,29 @@ void LLThreadLocalPtrBase::initAllThreadLocalStorage()
{
if (!sInitialized)
{
- sInitialized = true;
for (LLInstanceTracker<LLThreadLocalPtrBase>::instance_iter it = beginInstances(), end_it = endInstances();
it != end_it;
++it)
{
(*it).initStorage();
}
+ sInitialized = true;
}
}
+void LLThreadLocalPtrBase::destroyAllThreadLocalStorage()
+{
+ if (sInitialized)
+ {
+ for (LLInstanceTracker<LLThreadLocalPtrBase>::instance_iter it = beginInstances(), end_it = endInstances();
+ it != end_it;
+ ++it)
+ {
+ (*it).destroyStorage();
+ }
+ sInitialized = false;
+ }
+}
//
//*******************************************************************************************************************************
diff --git a/indra/llcommon/llapr.h b/indra/llcommon/llapr.h
index eb0bf627a0a..830e0a33fab 100644
--- a/indra/llcommon/llapr.h
+++ b/indra/llcommon/llapr.h
@@ -259,16 +259,19 @@ class LL_COMMON_API LLAPRFile : boost::noncopyable
class LLThreadLocalPtrBase : LLInstanceTracker<LLThreadLocalPtrBase>
{
public:
- LLThreadLocalPtrBase(void (*cleanup_func)(void*) );
+ LLThreadLocalPtrBase();
LLThreadLocalPtrBase(const LLThreadLocalPtrBase& other);
~LLThreadLocalPtrBase();
+ static void initAllThreadLocalStorage();
+ static void destroyAllThreadLocalStorage();
+
protected:
- friend void LL_COMMON_API ll_init_apr();
void set(void* value);
LL_FORCE_INLINE void* get()
{
+ llassert(sInitialized);
void* ptr;
//apr_status_t result =
apr_threadkey_private_get(&ptr, mThreadKey);
@@ -294,13 +297,9 @@ class LLThreadLocalPtrBase : LLInstanceTracker<LLThreadLocalPtrBase>
}
void initStorage();
-
void destroyStorage();
- static void initAllThreadLocalStorage();
-
-private:
- void (*mCleanupFunc)(void*);
+protected:
apr_threadkey_t* mThreadKey;
static bool sInitialized;
};
@@ -311,10 +310,10 @@ class LLThreadLocalPtr : public LLThreadLocalPtrBase
public:
LLThreadLocalPtr()
- : LLThreadLocalPtrBase(&cleanup)
+ : LLThreadLocalPtrBase()
{}
- LLThreadLocalPtr(T* value)
+ explicit LLThreadLocalPtr(T* value)
: LLThreadLocalPtrBase(&cleanup)
{
set(value);
@@ -327,7 +326,7 @@ class LLThreadLocalPtr : public LLThreadLocalPtrBase
set(other.get());
}
- T* get()
+ LL_FORCE_INLINE T* get()
{
return (T*)LLThreadLocalPtrBase::get();
}
@@ -363,13 +362,11 @@ class LLThreadLocalPtr : public LLThreadLocalPtrBase
return *this;
}
-private:
-
- static void cleanup(void* ptr)
+ bool operator ==(T* other)
{
- delete reinterpret_cast<T*>(ptr);
+ if (!sInitialized) return false;
+ return get() == other;
}
-
};
/**
diff --git a/indra/llcommon/llcommon.cpp b/indra/llcommon/llcommon.cpp
index 512e7da8400..c149a1fe5cc 100644
--- a/indra/llcommon/llcommon.cpp
+++ b/indra/llcommon/llcommon.cpp
@@ -45,15 +45,12 @@ void LLCommon::initClass()
LLTimer::initClass();
LLThreadSafeRefCount::initThreadSafeRefCount();
LLTrace::init();
-// LLWorkerThread::initClass();
-// LLFrameCallbackManager::initClass();
}
//static
void LLCommon::cleanupClass()
{
-// LLFrameCallbackManager::cleanupClass();
-// LLWorkerThread::cleanupClass();
+ LLTrace::cleanup();
LLThreadSafeRefCount::cleanupThreadSafeRefCount();
LLTimer::cleanupClass();
if (sAprInitialized)
@@ -62,5 +59,4 @@ void LLCommon::cleanupClass()
sAprInitialized = FALSE;
}
LLMemory::cleanupClass();
- LLTrace::cleanup();
}
diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp
index de1f0801a12..2e6eb085dcc 100644
--- a/indra/llcommon/llthread.cpp
+++ b/indra/llcommon/llthread.cpp
@@ -66,7 +66,7 @@ U32 __thread LLThread::sThreadID = 0;
#endif
U32 LLThread::sIDIter = 0;
-LLThreadLocalPtr<LLTrace::ThreadTraceData> LLThread::sTraceData;
+LLThreadLocalPtr<LLTrace::SlaveThreadTrace> LLThread::sTraceData;
LL_COMMON_API void assert_main_thread()
@@ -99,6 +99,8 @@ void *APR_THREAD_FUNC LLThread::staticRun(apr_thread_t *apr_threadp, void *datap
// We're done with the run function, this thread is done executing now.
threadp->mStatus = STOPPED;
+ delete sTraceData.get();
+
return NULL;
}
@@ -108,6 +110,7 @@ LLThread::LLThread(const std::string& name, apr_pool_t *poolp) :
mAPRThreadp(NULL),
mStatus(STOPPED)
{
+
mID = ++sIDIter;
// Thread creation probably CAN be paranoid about APR being initialized, if necessary
diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h
index fed111b0e42..6889fab2c19 100644
--- a/indra/llcommon/llthread.h
+++ b/indra/llcommon/llthread.h
@@ -105,7 +105,7 @@ class LL_COMMON_API LLThread
EThreadStatus mStatus;
U32 mID;
- static LLThreadLocalPtr<LLTrace::ThreadTraceData> sTraceData;
+ static LLThreadLocalPtr<LLTrace::SlaveThreadTrace> sTraceData;
//a local apr_pool for APRFile operations in this thread. If it exists, LLAPRFile::sAPRFilePoolp should not be used.
//Note: this pool is used by APRFile ONLY, do NOT use it for any other purposes.
diff --git a/indra/llcommon/lltrace.cpp b/indra/llcommon/lltrace.cpp
index 501414ebf34..8cedcafd10e 100644
--- a/indra/llcommon/lltrace.cpp
+++ b/indra/llcommon/lltrace.cpp
@@ -31,27 +31,37 @@
namespace LLTrace
{
-BlockTimer::Recorder::StackEntry BlockTimer::sCurRecorder;
-
-MasterThreadTrace *gMasterThreadTrace = NULL;
-LLThreadLocalPtr<ThreadTraceData> gCurThreadTrace;
+static MasterThreadTrace* gMasterThreadTrace = NULL;
void init()
{
gMasterThreadTrace = new MasterThreadTrace();
- gCurThreadTrace = gMasterThreadTrace;
}
void cleanup()
{
delete gMasterThreadTrace;
+ gMasterThreadTrace = NULL;
}
+BlockTimer::Recorder::StackEntry BlockTimer::sCurRecorder;
+
+
+
+MasterThreadTrace& getMasterThreadTrace()
+{
+ llassert(gMasterThreadTrace != NULL);
+ return *gMasterThreadTrace;
+}
+
+
+
///////////////////////////////////////////////////////////////////////
// Sampler
///////////////////////////////////////////////////////////////////////
+
void Sampler::stop()
{
getThreadTrace()->deactivate(this);
@@ -71,15 +81,15 @@ class ThreadTraceData* Sampler::getThreadTrace()
// MasterThreadTrace
///////////////////////////////////////////////////////////////////////
-void MasterThreadTrace::pullFromWorkerThreads()
+void MasterThreadTrace::pullFromSlaveThreads()
{
LLMutexLock lock(&mSlaveListMutex);
- for (worker_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
+ for (slave_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
it != end_it;
++it)
{
- it->mWorkerTrace->mSharedData.copyTo(it->mSamplerStorage);
+ it->mSlaveTrace->mSharedData.copyTo(it->mSamplerStorage);
}
}
@@ -87,18 +97,18 @@ void MasterThreadTrace::addSlaveThread( class SlaveThreadTrace* child )
{
LLMutexLock lock(&mSlaveListMutex);
- mSlaveThreadTraces.push_back(WorkerThreadTraceProxy(child));
+ mSlaveThreadTraces.push_back(SlaveThreadTraceProxy(child));
}
void MasterThreadTrace::removeSlaveThread( class SlaveThreadTrace* child )
{
LLMutexLock lock(&mSlaveListMutex);
- for (worker_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
+ for (slave_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
it != end_it;
++it)
{
- if (it->mWorkerTrace == child)
+ if (it->mSlaveTrace == child)
{
mSlaveThreadTraces.erase(it);
break;
@@ -107,3 +117,4 @@ void MasterThreadTrace::removeSlaveThread( class SlaveThreadTrace* child )
}
}
+
diff --git a/indra/llcommon/lltrace.h b/indra/llcommon/lltrace.h
index 3af05d67f9d..e4bec0a6440 100644
--- a/indra/llcommon/lltrace.h
+++ b/indra/llcommon/lltrace.h
@@ -44,8 +44,7 @@ namespace LLTrace
void init();
void cleanup();
- extern class MasterThreadTrace *gMasterThreadTrace;
- extern LLThreadLocalPtr<class ThreadTraceData> gCurThreadTrace;
+ class MasterThreadTrace& getMasterThreadTrace();
// one per thread per type
template<typename ACCUMULATOR>
@@ -59,8 +58,8 @@ namespace LLTrace
: mStorageSize(64),
mNextStorageSlot(0),
mStorage(new ACCUMULATOR[DEFAULT_ACCUMULATOR_BUFFER_SIZE])
- {
- }
+ {}
+
public:
AccumulatorBuffer(const AccumulatorBuffer& other = getDefaultBuffer())
@@ -69,6 +68,15 @@ namespace LLTrace
mNextStorageSlot(other.mNextStorageSlot)
{}
+ ~AccumulatorBuffer()
+ {
+ if (sPrimaryStorage == mStorage)
+ {
+ //TODO pick another primary?
+ sPrimaryStorage = NULL;
+ }
+ }
+
LL_FORCE_INLINE ACCUMULATOR& operator[](size_t index)
{
return mStorage[index];
@@ -353,7 +361,8 @@ namespace LLTrace
mTimers(other.mTimers)
{}
- ~Sampler() {}
+ ~Sampler()
+ {}
void makePrimary()
{
@@ -453,21 +462,21 @@ namespace LLTrace
void addSlaveThread(class SlaveThreadTrace* child);
void removeSlaveThread(class SlaveThreadTrace* child);
- // call this periodically to gather stats data from worker threads
- void pullFromWorkerThreads();
+ // call this periodically to gather stats data from slave threads
+ void pullFromSlaveThreads();
private:
- struct WorkerThreadTraceProxy
+ struct SlaveThreadTraceProxy
{
- WorkerThreadTraceProxy(class SlaveThreadTrace* trace)
- : mWorkerTrace(trace)
+ SlaveThreadTraceProxy(class SlaveThreadTrace* trace)
+ : mSlaveTrace(trace)
{}
- class SlaveThreadTrace* mWorkerTrace;
+ class SlaveThreadTrace* mSlaveTrace;
Sampler mSamplerStorage;
};
- typedef std::list<WorkerThreadTraceProxy> worker_thread_trace_list_t;
+ typedef std::list<SlaveThreadTraceProxy> slave_thread_trace_list_t;
- worker_thread_trace_list_t mSlaveThreadTraces;
+ slave_thread_trace_list_t mSlaveThreadTraces;
LLMutex mSlaveListMutex;
};
@@ -475,17 +484,16 @@ namespace LLTrace
{
public:
explicit
- SlaveThreadTrace(MasterThreadTrace& master_trace = *gMasterThreadTrace)
- : mMaster(master_trace),
- ThreadTraceData(master_trace),
+ SlaveThreadTrace()
+ : ThreadTraceData(getMasterThreadTrace()),
mSharedData(mPrimarySampler)
{
- master_trace.addSlaveThread(this);
+ getMasterThreadTrace().addSlaveThread(this);
}
~SlaveThreadTrace()
{
- mMaster.removeSlaveThread(this);
+ getMasterThreadTrace().removeSlaveThread(this);
}
// call this periodically to gather stats data for master thread to consume
@@ -494,7 +502,7 @@ namespace LLTrace
mSharedData.copyFrom(mPrimarySampler);
}
- MasterThreadTrace& mMaster;
+ MasterThreadTrace* mMaster;
// this data is accessed by other threads, so give it a 64 byte alignment
// to avoid false sharing on most x86 processors
@@ -529,7 +537,6 @@ namespace LLTrace
SharedData mSharedData;
};
-
class TimeInterval
{
public:
diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp
index e255d65b43b..be7aae4eb7d 100644
--- a/indra/llimage/tests/llimageworker_test.cpp
+++ b/indra/llimage/tests/llimageworker_test.cpp
@@ -114,11 +114,13 @@ namespace tut
// Constructor and destructor of the test wrapper
imagedecodethread_test()
{
+ LLTrace::init();
mThread = NULL;
}
~imagedecodethread_test()
{
delete mThread;
+ LLTrace::cleanup();
}
};
@@ -136,6 +138,8 @@ namespace tut
imagerequest_test()
{
done = false;
+ LLTrace::init();
+
mRequest = new LLImageDecodeThread::ImageRequest(0, 0,
LLQueuedThread::PRIORITY_NORMAL, 0, FALSE,
new responder_test(&done));
@@ -145,6 +149,7 @@ namespace tut
// We should delete the object *but*, because its destructor is protected, that cannot be
// done from outside an LLImageDecodeThread instance... So we leak memory here... It's fine...
//delete mRequest;
+ LLTrace::cleanup();
}
};
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index dc8580fe697..2b66c6aa265 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -512,15 +512,10 @@ int main(int argc, char **argv)
ctype_workaround();
#endif
- apr_initialize();
- apr_pool_t* pool = NULL;
- if(APR_SUCCESS != apr_pool_create(&pool, NULL))
- {
- std::cerr << "Unable to initialize pool" << std::endl;
- return 1;
- }
+ ll_init_apr();
+
apr_getopt_t* os = NULL;
- if(APR_SUCCESS != apr_getopt_init(&os, pool, argc, argv))
+ if(APR_SUCCESS != apr_getopt_init(&os, gAPRPoolp, argc, argv))
{
std::cerr << "apr_getopt_init() failed" << std::endl;
return 1;
@@ -602,7 +597,7 @@ int main(int argc, char **argv)
if (LOGFAIL)
{
LLError::ELevel level = LLError::decodeLevel(LOGFAIL);
- replayer.reset(new LLReplayLogReal(level, pool));
+ replayer.reset(new LLReplayLogReal(level, gAPRPoolp));
}
else
{
@@ -646,7 +641,7 @@ int main(int argc, char **argv)
s.close();
}
- apr_terminate();
+ ll_cleanup_apr();
int retval = (success ? 0 : 1);
return retval;
--
GitLab