livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

EXT-5055 LLInstanceTracker promotes some dangerous patterns - detect them

master
Tofu Linden 2010-02-06 21:38:57 +00:00
parent 346cabd557
commit a4d224ff93
7 changed files with 150 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
indra/llcommon/lleventtimer.cpp
View File

@ -42,7 +42,6 @@
// LLEventTimer Implementation
//
//////////////////////////////////////////////////////////////////////////////
bool LLEventTimer::sInTickLoop = false;
LLEventTimer::LLEventTimer(F32 period)
: mEventTimer()
@ -59,27 +58,28 @@ LLEventTimer::LLEventTimer(const LLDate& time)
LLEventTimer::~LLEventTimer()
{
llassert(!LLEventTimer::sInTickLoop); // this LLEventTimer was destroyed from within its own tick() function - bad. if you want tick() to cause destruction of its own timer, make it return true.
}
//static
void LLEventTimer::updateClass()
{
std::list<LLEventTimer*> completed_timers;
LLEventTimer::sInTickLoop = true;
for (instance_iter iter = beginInstances(); iter != endInstances(); )
{
LLEventTimer& timer = *iter++;
F32 et = timer.mEventTimer.getElapsedTimeF32();
if (timer.mEventTimer.getStarted() && et > timer.mPeriod) {
timer.mEventTimer.reset();
if ( timer.tick() )
{
completed_timers.push_back( &timer );
LLInstanceTrackerScopedGuard guard;
for (instance_iter iter = guard.beginInstances(); iter != guard.endInstances(); )
{
LLEventTimer& timer = *iter++;
F32 et = timer.mEventTimer.getElapsedTimeF32();
if (timer.mEventTimer.getStarted() && et > timer.mPeriod) {
timer.mEventTimer.reset();
if ( timer.tick() )
{
completed_timers.push_back( &timer );
}
}
}
}
LLEventTimer::sInTickLoop = false;
if ( completed_timers.size() > 0 )
{

1
indra/llcommon/lleventtimer.h
View File

@ -55,7 +55,6 @@ public:
protected:
LLTimer mEventTimer;
F32 mPeriod;
static bool sInTickLoop;
};
#endif //LL_EVENTTIMER_H

111
indra/llcommon/llfasttimer_class.cpp
View File

@ -218,9 +218,10 @@ LLFastTimer::DeclareTimer::DeclareTimer(const std::string& name)
// static
void LLFastTimer::DeclareTimer::updateCachedPointers()
{
DeclareTimer::LLInstanceTrackerScopedGuard guard;
// propagate frame state pointers to timer declarations
for (DeclareTimer::instance_iter it = DeclareTimer::beginInstances();
it != DeclareTimer::endInstances();
for (DeclareTimer::instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
// update cached pointer
@ -371,20 +372,23 @@ void LLFastTimer::NamedTimer::buildHierarchy()
if (sCurFrameIndex < 0 ) return;
// set up initial tree
for (instance_iter it = NamedTimer::beginInstances();
it != endInstances();
++it)
{
NamedTimer& timer = *it;
if (&timer == NamedTimerFactory::instance().getRootTimer()) continue;
// bootstrap tree construction by attaching to last timer to be on stack
// when this timer was called
if (timer.getFrameState().mLastCaller && timer.mParent == NamedTimerFactory::instance().getRootTimer())
NamedTimer::LLInstanceTrackerScopedGuard guard;
for (instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
timer.setParent(timer.getFrameState().mLastCaller->mTimer);
// no need to push up tree on first use, flag can be set spuriously
timer.getFrameState().mMoveUpTree = false;
NamedTimer& timer = *it;
if (&timer == NamedTimerFactory::instance().getRootTimer()) continue;
// bootstrap tree construction by attaching to last timer to be on stack
// when this timer was called
if (timer.getFrameState().mLastCaller && timer.mParent == NamedTimerFactory::instance().getRootTimer())
{
timer.setParent(timer.getFrameState().mLastCaller->mTimer);
// no need to push up tree on first use, flag can be set spuriously
timer.getFrameState().mMoveUpTree = false;
}
}
}
@ -486,18 +490,21 @@ void LLFastTimer::NamedTimer::resetFrame()
F64 total_time = 0;
LLSD sd;
for (NamedTimer::instance_iter it = NamedTimer::beginInstances();
it != NamedTimer::endInstances();
++it)
{
NamedTimer& timer = *it;
FrameState& info = timer.getFrameState();
sd[timer.getName()]["Time"] = (LLSD::Real) (info.mSelfTimeCounter*iclock_freq);
sd[timer.getName()]["Calls"] = (LLSD::Integer) info.mCalls;
// computing total time here because getting the root timer's getCountHistory
// doesn't work correctly on the first frame
total_time = total_time + info.mSelfTimeCounter * iclock_freq;
NamedTimer::LLInstanceTrackerScopedGuard guard;
for (NamedTimer::instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
NamedTimer& timer = *it;
FrameState& info = timer.getFrameState();
sd[timer.getName()]["Time"] = (LLSD::Real) (info.mSelfTimeCounter*iclock_freq);
sd[timer.getName()]["Calls"] = (LLSD::Integer) info.mCalls;
// computing total time here because getting the root timer's getCountHistory
// doesn't work correctly on the first frame
total_time = total_time + info.mSelfTimeCounter * iclock_freq;
}
}
sd["Total"]["Time"] = (LLSD::Real) total_time;
@ -531,21 +538,24 @@ void LLFastTimer::NamedTimer::resetFrame()
DeclareTimer::updateCachedPointers();
// reset for next frame
for (NamedTimer::instance_iter it = NamedTimer::beginInstances();
it != NamedTimer::endInstances();
++it)
{
NamedTimer& timer = *it;
FrameState& info = timer.getFrameState();
info.mSelfTimeCounter = 0;
info.mCalls = 0;
info.mLastCaller = NULL;
info.mMoveUpTree = false;
// update parent pointer in timer state struct
if (timer.mParent)
NamedTimer::LLInstanceTrackerScopedGuard guard;
for (NamedTimer::instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
info.mParent = &timer.mParent->getFrameState();
NamedTimer& timer = *it;
FrameState& info = timer.getFrameState();
info.mSelfTimeCounter = 0;
info.mCalls = 0;
info.mLastCaller = NULL;
info.mMoveUpTree = false;
// update parent pointer in timer state struct
if (timer.mParent)
{
info.mParent = &timer.mParent->getFrameState();
}
}
}
@ -575,20 +585,23 @@ void LLFastTimer::NamedTimer::reset()
}
// reset all history
for (NamedTimer::instance_iter it = NamedTimer::beginInstances();
it != NamedTimer::endInstances();
++it)
{
NamedTimer& timer = *it;
if (&timer != NamedTimerFactory::instance().getRootTimer())
NamedTimer::LLInstanceTrackerScopedGuard guard;
for (NamedTimer::instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
timer.setParent(NamedTimerFactory::instance().getRootTimer());
NamedTimer& timer = *it;
if (&timer != NamedTimerFactory::instance().getRootTimer())
{
timer.setParent(NamedTimerFactory::instance().getRootTimer());
}
timer.mCountAverage = 0;
timer.mCallAverage = 0;
memset(timer.mCountHistory, 0, sizeof(U32) * HISTORY_NUM);
memset(timer.mCallHistory, 0, sizeof(U32) * HISTORY_NUM);
}
timer.mCountAverage = 0;
timer.mCallAverage = 0;
memset(timer.mCountHistory, 0, sizeof(U32) * HISTORY_NUM);
memset(timer.mCallHistory, 0, sizeof(U32) * HISTORY_NUM);
}
sLastFrameIndex = 0;

69
indra/llcommon/llinstancetracker.h
View File

@ -98,7 +98,10 @@ private:
mKey = key;
getMap_()[key] = static_cast<T*>(this);
}
void remove_() { getMap_().erase(mKey); }
void remove_()
{
getMap_().erase(mKey);
}
static InstanceMap& getMap_()
{
@ -129,31 +132,65 @@ public:
/// for completeness of analogy with the generic implementation
static T* getInstance(T* k) { return k; }
static key_iter beginKeys() { return getSet_().begin(); }
static key_iter endKeys() { return getSet_().end(); }
static instance_iter beginInstances() { return instance_iter(getSet_().begin()); }
static instance_iter endInstances() { return instance_iter(getSet_().end()); }
static S32 instanceCount() { return getSet_().size(); }
// Instantiate this to get access to iterators for this type. It's a 'guard' in the sense
// that it treats deletes of this type as errors as long as there is an instance of
// this class alive in scope somewhere (i.e. deleting while iterating is bad).
class LLInstanceTrackerScopedGuard
{
public:
LLInstanceTrackerScopedGuard()
{
++sIterationNestDepth;
}
~LLInstanceTrackerScopedGuard()
{
--sIterationNestDepth;
}
static instance_iter beginInstances() { return instance_iter(getSet_().begin()); }
static instance_iter endInstances() { return instance_iter(getSet_().end()); }
static key_iter beginKeys() { return getSet_().begin(); }
static key_iter endKeys() { return getSet_().end(); }
};
protected:
LLInstanceTracker() { getSet_().insert(static_cast<T*>(this)); }
virtual ~LLInstanceTracker() { getSet_().erase(static_cast<T*>(this)); }
LLInstanceTracker()
{
// it's safe but unpredictable to create instances of this type while all instances are being iterated over. I hate unpredictable. This assert will probably be turned on early in the next development cycle.
//llassert(sIterationNestDepth == 0);
getSet_().insert(static_cast<T*>(this));
}
virtual ~LLInstanceTracker()
{
// it's unsafe to delete instances of this type while all instances are being iterated over.
llassert(sIterationNestDepth == 0);
getSet_().erase(static_cast<T*>(this));
}
LLInstanceTracker(const LLInstanceTracker& other) { getSet_().insert(static_cast<T*>(this)); }
LLInstanceTracker(const LLInstanceTracker& other)
{
//llassert(sIterationNestDepth == 0);
getSet_().insert(static_cast<T*>(this));
}
static InstanceSet& getSet_() // called after getReady() but before go()
{
if (! sInstances)
{
sInstances = new InstanceSet;
}
return *sInstances;
}
static InstanceSet& getSet_()
{
if (! sInstances)
{
sInstances = new InstanceSet;
}
return *sInstances;
}
static InstanceSet* sInstances;
static S32 sIterationNestDepth;
};
template <typename T, typename KEY> typename LLInstanceTracker<T, KEY>::InstanceMap* LLInstanceTracker<T, KEY>::sInstances = NULL;
template <typename T> typename LLInstanceTracker<T, T*>::InstanceSet* LLInstanceTracker<T, T*>::sInstances = NULL;
template <typename T> S32 LLInstanceTracker<T, T*>::sIterationNestDepth = 0;
#endif

28
indra/llcommon/tests/llinstancetracker_test.cpp
View File

@ -138,23 +138,29 @@ namespace tut
keys.insert(&one);
keys.insert(&two);
keys.insert(&three);
for (Unkeyed::key_iter ki(Unkeyed::beginKeys()), kend(Unkeyed::endKeys());
ki != kend; ++ki)
{
ensure_equals("spurious key", keys.erase(*ki), 1);
}
{
Unkeyed::LLInstanceTrackerScopedGuard guard;
for (Unkeyed::key_iter ki(guard.beginKeys()), kend(guard.endKeys());
ki != kend; ++ki)
{
ensure_equals("spurious key", keys.erase(*ki), 1);
}
}
ensure_equals("unreported key", keys.size(), 0);
KeySet instances;
instances.insert(&one);
instances.insert(&two);
instances.insert(&three);
for (Unkeyed::instance_iter ii(Unkeyed::beginInstances()), iend(Unkeyed::endInstances());
ii != iend; ++ii)
{
Unkeyed& ref = *ii;
ensure_equals("spurious instance", instances.erase(&ref), 1);
}
{
Unkeyed::LLInstanceTrackerScopedGuard guard;
for (Unkeyed::instance_iter ii(guard.beginInstances()), iend(guard.endInstances());
ii != iend; ++ii)
{
Unkeyed& ref = *ii;
ensure_equals("spurious instance", instances.erase(&ref), 1);
}
}
ensure_equals("unreported instance", instances.size(), 0);
}
} // namespace tut

5
indra/llui/lllayoutstack.cpp
View File

@ -816,7 +816,10 @@ void LLLayoutStack::calcMinExtents()
//static
void LLLayoutStack::updateClass()
{
for (LLLayoutStack::instance_iter it = beginInstances(); it != endInstances(); ++it)
LLInstanceTrackerScopedGuard guard;
for (LLLayoutStack::instance_iter it = guard.beginInstances();
it != guard.endInstances();
++it)
{
it->updateLayout();
}

3
indra/newview/llnamelistctrl.cpp
View File

@ -356,8 +356,9 @@ void LLNameListCtrl::refresh(const LLUUID& id, const std::string& first,
void LLNameListCtrl::refreshAll(const LLUUID& id, const std::string& first,
const std::string& last, BOOL is_group)
{
LLInstanceTrackerScopedGuard guard;
LLInstanceTracker<LLNameListCtrl>::instance_iter it;
for (it = beginInstances(); it != endInstances(); ++it)
for (it = guard.beginInstances(); it != guard.endInstances(); ++it)
{
LLNameListCtrl& ctrl = *it;
ctrl.refresh(id, first, last, is_group);