From 14d942f16b284ca5180b29f3c912d158c0d0cdd0 Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Thu, 4 Feb 2010 11:08:49 +0000 Subject: A flag+assert to help track bad behaviour in LLEventTimer, especially EXT-4754 --- indra/llcommon/lltimer.cpp | 6 ++++++ indra/llcommon/lltimer.h | 1 + 2 files changed, 7 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lltimer.cpp b/indra/llcommon/lltimer.cpp index ef3e8dbc94..63634117b5 100644 --- a/indra/llcommon/lltimer.cpp +++ b/indra/llcommon/lltimer.cpp @@ -565,19 +565,23 @@ LLEventTimer::LLEventTimer(F32 period) : mEventTimer() { mPeriod = period; + mBusy = false; } LLEventTimer::LLEventTimer(const LLDate& time) : mEventTimer() { mPeriod = (F32)(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch()); + mBusy = false; } LLEventTimer::~LLEventTimer() { + llassert(!mBusy); // this LLEventTimer was destroyed from its own tick() function - bad. } +//static void LLEventTimer::updateClass() { std::list completed_timers; @@ -587,10 +591,12 @@ void LLEventTimer::updateClass() F32 et = timer.mEventTimer.getElapsedTimeF32(); if (timer.mEventTimer.getStarted() && et > timer.mPeriod) { timer.mEventTimer.reset(); + timer.mBusy = true; if ( timer.tick() ) { completed_timers.push_back( &timer ); } + timer.mBusy = false; } } diff --git a/indra/llcommon/lltimer.h b/indra/llcommon/lltimer.h index d009c0f5f7..4d995d5bba 100644 --- a/indra/llcommon/lltimer.h +++ b/indra/llcommon/lltimer.h @@ -188,6 +188,7 @@ public: protected: LLTimer mEventTimer; F32 mPeriod; + bool mBusy; }; U64 LL_COMMON_API totalTime(); // Returns current system time in microseconds -- cgit v1.2.3 From 2c30ccf34d518ccedd0b3ffdeb2ba1da8d140a1d Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Thu, 4 Feb 2010 11:24:14 +0000 Subject: EXT-4754 Crash in LLEventTimer::updateClass --- indra/llcommon/lltimer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lltimer.cpp b/indra/llcommon/lltimer.cpp index 63634117b5..21e165ebc9 100644 --- a/indra/llcommon/lltimer.cpp +++ b/indra/llcommon/lltimer.cpp @@ -578,7 +578,7 @@ LLEventTimer::LLEventTimer(const LLDate& time) LLEventTimer::~LLEventTimer() { - llassert(!mBusy); // this LLEventTimer was destroyed from its own tick() function - bad. + llassert(!mBusy); // 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 -- cgit v1.2.3 From ec076c97feaa2976520e28fd1c1b23099be656d7 Mon Sep 17 00:00:00 2001 From: "Mark Palange (Mani)" Date: Fri, 5 Feb 2010 18:15:31 -0800 Subject: EXT-4754 Fix for LLEventTimer::updateClass crash. Ugh. Update on Tofu's patch for this bug. Reviewed by Richard --- indra/llcommon/lltimer.cpp | 9 ++++----- indra/llcommon/lltimer.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lltimer.cpp b/indra/llcommon/lltimer.cpp index 21e165ebc9..900e122ea5 100644 --- a/indra/llcommon/lltimer.cpp +++ b/indra/llcommon/lltimer.cpp @@ -560,45 +560,44 @@ void secondsToTimecodeString(F32 current_time, std::string& tcstring) // LLEventTimer Implementation // ////////////////////////////////////////////////////////////////////////////// +bool LLEventTimer::sInTickLoop = false; LLEventTimer::LLEventTimer(F32 period) : mEventTimer() { mPeriod = period; - mBusy = false; } LLEventTimer::LLEventTimer(const LLDate& time) : mEventTimer() { mPeriod = (F32)(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch()); - mBusy = false; } LLEventTimer::~LLEventTimer() { - llassert(!mBusy); // 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. + 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 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(); - timer.mBusy = true; if ( timer.tick() ) { completed_timers.push_back( &timer ); } - timer.mBusy = false; } } + LLEventTimer::sInTickLoop = false; if ( completed_timers.size() > 0 ) { diff --git a/indra/llcommon/lltimer.h b/indra/llcommon/lltimer.h index 4d995d5bba..63e8121b58 100644 --- a/indra/llcommon/lltimer.h +++ b/indra/llcommon/lltimer.h @@ -188,7 +188,7 @@ public: protected: LLTimer mEventTimer; F32 mPeriod; - bool mBusy; + static bool sInTickLoop; }; U64 LL_COMMON_API totalTime(); // Returns current system time in microseconds -- cgit v1.2.3 From 4c7ae59996c2743152928abd81831a2ab5e788b6 Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Sat, 6 Feb 2010 18:59:44 +0000 Subject: move LLEventTimer into its own source module. everyone includes it, almost no-one wants it. now I can dick with it a bit without rebuilding the world, at least. :) --- indra/llcommon/CMakeLists.txt | 1 + indra/llcommon/llapp.cpp | 2 +- indra/llcommon/lleventtimer.cpp | 95 +++++++++++++++++++++++++++++++++++++++++ indra/llcommon/lleventtimer.h | 61 ++++++++++++++++++++++++++ indra/llcommon/lllivefile.cpp | 2 +- indra/llcommon/lltimer.cpp | 56 ------------------------ indra/llcommon/lltimer.h | 22 ---------- 7 files changed, 159 insertions(+), 80 deletions(-) create mode 100644 indra/llcommon/lleventtimer.cpp create mode 100644 indra/llcommon/lleventtimer.h (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 9ead183a9e..5941bc5bb5 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -50,6 +50,7 @@ set(llcommon_SOURCE_FILES lleventdispatcher.cpp lleventfilter.cpp llevents.cpp + lleventtimer.cpp llfasttimer_class.cpp llfile.cpp llfindlocale.cpp diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 968b92d1e7..6b2d1b7c20 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -41,7 +41,7 @@ #include "lllivefile.h" #include "llmemory.h" #include "llstl.h" // for DeletePointer() -#include "lltimer.h" +#include "lleventtimer.h" // // Signal handling diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp new file mode 100644 index 0000000000..b2fdf11197 --- /dev/null +++ b/indra/llcommon/lleventtimer.cpp @@ -0,0 +1,95 @@ +/** + * @file lleventtimer.cpp + * @brief Cross-platform objects for doing timing + * + * $LicenseInfo:firstyear=2000&license=viewergpl$ + * + * Copyright (c) 2000-2009, Linden Research, Inc. + * + * Second Life Viewer Source Code + * The source code in this file ("Source Code") is provided by Linden Lab + * to you under the terms of the GNU General Public License, version 2.0 + * ("GPL"), unless you have obtained a separate licensing agreement + * ("Other License"), formally executed by you and Linden Lab. Terms of + * the GPL can be found in doc/GPL-license.txt in this distribution, or + * online at http://secondlifegrid.net/programs/open_source/licensing/gplv2 + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution, or + * online at + * http://secondlifegrid.net/programs/open_source/licensing/flossexception + * + * By copying, modifying or distributing this software, you acknowledge + * that you have read and understood your obligations described above, + * and agree to abide by those obligations. + * + * ALL LINDEN LAB SOURCE CODE IS PROVIDED "AS IS." LINDEN LAB MAKES NO + * WARRANTIES, EXPRESS, IMPLIED OR OTHERWISE, REGARDING ITS ACCURACY, + * COMPLETENESS OR PERFORMANCE. + * $/LicenseInfo$ + */ + +#include "linden_common.h" + +#include "lleventtimer.h" + +#include "u64.h" + + +////////////////////////////////////////////////////////////////////////////// +// +// LLEventTimer Implementation +// +////////////////////////////////////////////////////////////////////////////// +bool LLEventTimer::sInTickLoop = false; + +LLEventTimer::LLEventTimer(F32 period) +: mEventTimer() +{ + mPeriod = period; +} + +LLEventTimer::LLEventTimer(const LLDate& time) +: mEventTimer() +{ + mPeriod = (F32)(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch()); +} + + +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 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 ); + } + } + } + LLEventTimer::sInTickLoop = false; + + if ( completed_timers.size() > 0 ) + { + for (std::list::iterator completed_iter = completed_timers.begin(); + completed_iter != completed_timers.end(); + completed_iter++ ) + { + delete *completed_iter; + } + } +} + + diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h new file mode 100644 index 0000000000..f792138feb --- /dev/null +++ b/indra/llcommon/lleventtimer.h @@ -0,0 +1,61 @@ +/** + * @file lleventtimer.h + * @brief Cross-platform objects for doing timing + * + * $LicenseInfo:firstyear=2000&license=viewergpl$ + * + * Copyright (c) 2000-2009, Linden Research, Inc. + * + * Second Life Viewer Source Code + * The source code in this file ("Source Code") is provided by Linden Lab + * to you under the terms of the GNU General Public License, version 2.0 + * ("GPL"), unless you have obtained a separate licensing agreement + * ("Other License"), formally executed by you and Linden Lab. Terms of + * the GPL can be found in doc/GPL-license.txt in this distribution, or + * online at http://secondlifegrid.net/programs/open_source/licensing/gplv2 + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution, or + * online at + * http://secondlifegrid.net/programs/open_source/licensing/flossexception + * + * By copying, modifying or distributing this software, you acknowledge + * that you have read and understood your obligations described above, + * and agree to abide by those obligations. + * + * ALL LINDEN LAB SOURCE CODE IS PROVIDED "AS IS." LINDEN LAB MAKES NO + * WARRANTIES, EXPRESS, IMPLIED OR OTHERWISE, REGARDING ITS ACCURACY, + * COMPLETENESS OR PERFORMANCE. + * $/LicenseInfo$ + */ + +#ifndef LL_EVENTTIMER_H +#define LL_EVENTTIMER_H + +#include "stdtypes.h" +#include "lldate.h" +#include "llinstancetracker.h" +#include "lltimer.h" + +// class for scheduling a function to be called at a given frequency (approximate, inprecise) +class LL_COMMON_API LLEventTimer : protected LLInstanceTracker +{ +public: + LLEventTimer(F32 period); // period is the amount of time between each call to tick() in seconds + LLEventTimer(const LLDate& time); + virtual ~LLEventTimer(); + + //function to be called at the supplied frequency + // Normally return FALSE; TRUE will delete the timer after the function returns. + virtual BOOL tick() = 0; + + static void updateClass(); + +protected: + LLTimer mEventTimer; + F32 mPeriod; + static bool sInTickLoop; +}; + +#endif //LL_EVENTTIMER_H diff --git a/indra/llcommon/lllivefile.cpp b/indra/llcommon/lllivefile.cpp index effda6c49c..5ca90d82ba 100644 --- a/indra/llcommon/lllivefile.cpp +++ b/indra/llcommon/lllivefile.cpp @@ -33,7 +33,7 @@ #include "lllivefile.h" #include "llframetimer.h" -#include "lltimer.h" +#include "lleventtimer.h" const F32 DEFAULT_CONFIG_FILE_REFRESH = 5.0f; diff --git a/indra/llcommon/lltimer.cpp b/indra/llcommon/lltimer.cpp index 900e122ea5..25b768079b 100644 --- a/indra/llcommon/lltimer.cpp +++ b/indra/llcommon/lltimer.cpp @@ -555,59 +555,3 @@ void secondsToTimecodeString(F32 current_time, std::string& tcstring) } -////////////////////////////////////////////////////////////////////////////// -// -// LLEventTimer Implementation -// -////////////////////////////////////////////////////////////////////////////// -bool LLEventTimer::sInTickLoop = false; - -LLEventTimer::LLEventTimer(F32 period) -: mEventTimer() -{ - mPeriod = period; -} - -LLEventTimer::LLEventTimer(const LLDate& time) -: mEventTimer() -{ - mPeriod = (F32)(time.secondsSinceEpoch() - LLDate::now().secondsSinceEpoch()); -} - - -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 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 ); - } - } - } - LLEventTimer::sInTickLoop = false; - - if ( completed_timers.size() > 0 ) - { - for (std::list::iterator completed_iter = completed_timers.begin(); - completed_iter != completed_timers.end(); - completed_iter++ ) - { - delete *completed_iter; - } - } -} - - diff --git a/indra/llcommon/lltimer.h b/indra/llcommon/lltimer.h index 63e8121b58..baba95bfa1 100644 --- a/indra/llcommon/lltimer.h +++ b/indra/llcommon/lltimer.h @@ -39,8 +39,6 @@ #include #include "stdtypes.h" -#include "lldate.h" -#include "llinstancetracker.h" #include #include @@ -171,26 +169,6 @@ LL_COMMON_API struct tm* utc_to_pacific_time(time_t utc_time, BOOL pacific_dayli LL_COMMON_API void microsecondsToTimecodeString(U64 current_time, std::string& tcstring); LL_COMMON_API void secondsToTimecodeString(F32 current_time, std::string& tcstring); -// class for scheduling a function to be called at a given frequency (approximate, inprecise) -class LL_COMMON_API LLEventTimer : protected LLInstanceTracker -{ -public: - LLEventTimer(F32 period); // period is the amount of time between each call to tick() in seconds - LLEventTimer(const LLDate& time); - virtual ~LLEventTimer(); - - //function to be called at the supplied frequency - // Normally return FALSE; TRUE will delete the timer after the function returns. - virtual BOOL tick() = 0; - - static void updateClass(); - -protected: - LLTimer mEventTimer; - F32 mPeriod; - static bool sInTickLoop; -}; - U64 LL_COMMON_API totalTime(); // Returns current system time in microseconds #endif -- cgit v1.2.3 From 346cabd557d921dea4f3a122df1e4f8eec4fc9f0 Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Sat, 6 Feb 2010 21:03:55 +0000 Subject: make the instancetracker unit test really, like, work. before I fiddle with a big pile of stuff. --- indra/llcommon/CMakeLists.txt | 1 - indra/llcommon/llinstancetracker.cpp | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 indra/llcommon/llinstancetracker.cpp (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 5941bc5bb5..4481d334b2 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -165,7 +165,6 @@ set(llcommon_HEADER_FILES llhttpstatuscodes.h llindexedqueue.h llinstancetracker.h - llinstancetracker.h llkeythrottle.h lllazy.h lllistenerwrapper.h diff --git a/indra/llcommon/llinstancetracker.cpp b/indra/llcommon/llinstancetracker.cpp new file mode 100644 index 0000000000..c962cb5be1 --- /dev/null +++ b/indra/llcommon/llinstancetracker.cpp @@ -0,0 +1,20 @@ +/** + * @file lllinstancetracker.cpp + * + * $LicenseInfo:firstyear=2009&license=viewergpl$ + * Copyright (c) 2009, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llinstancetracker.h" +// STL headers +// std headers +// external library headers +// other Linden headers + +// llinstancetracker.h is presently header-only. This file exists only because our CMake +// test macro ADD_BUILD_TEST requires it. +int dummy = 0; -- cgit v1.2.3 From a4d224ff9316bf3b99e17f72f844d91e0ef80cc7 Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Sat, 6 Feb 2010 21:38:57 +0000 Subject: EXT-5055 LLInstanceTracker promotes some dangerous patterns - detect them --- indra/llcommon/lleventtimer.cpp | 24 ++--- indra/llcommon/lleventtimer.h | 1 - indra/llcommon/llfasttimer_class.cpp | 111 +++++++++++++----------- indra/llcommon/llinstancetracker.h | 69 +++++++++++---- indra/llcommon/tests/llinstancetracker_test.cpp | 28 +++--- 5 files changed, 144 insertions(+), 89 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index b2fdf11197..d44e7ec1e6 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -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 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 ) { diff --git a/indra/llcommon/lleventtimer.h b/indra/llcommon/lleventtimer.h index f792138feb..5181cce52d 100644 --- a/indra/llcommon/lleventtimer.h +++ b/indra/llcommon/lleventtimer.h @@ -55,7 +55,6 @@ public: protected: LLTimer mEventTimer; F32 mPeriod; - static bool sInTickLoop; }; #endif //LL_EVENTTIMER_H diff --git a/indra/llcommon/llfasttimer_class.cpp b/indra/llcommon/llfasttimer_class.cpp index 6d8d81e114..2e5edb1f3b 100644 --- a/indra/llcommon/llfasttimer_class.cpp +++ b/indra/llcommon/llfasttimer_class.cpp @@ -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; diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h index 11fe523651..9df7998273 100644 --- a/indra/llcommon/llinstancetracker.h +++ b/indra/llcommon/llinstancetracker.h @@ -98,7 +98,10 @@ private: mKey = key; getMap_()[key] = static_cast(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(this)); } - virtual ~LLInstanceTracker() { getSet_().erase(static_cast(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(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(this)); + } - LLInstanceTracker(const LLInstanceTracker& other) { getSet_().insert(static_cast(this)); } + LLInstanceTracker(const LLInstanceTracker& other) + { + //llassert(sIterationNestDepth == 0); + getSet_().insert(static_cast(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 LLInstanceTracker::InstanceMap* LLInstanceTracker::sInstances = NULL; template typename LLInstanceTracker::InstanceSet* LLInstanceTracker::sInstances = NULL; +template S32 LLInstanceTracker::sIterationNestDepth = 0; #endif diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp index 7415f2d33b..4bb3ec2922 100644 --- a/indra/llcommon/tests/llinstancetracker_test.cpp +++ b/indra/llcommon/tests/llinstancetracker_test.cpp @@ -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 -- cgit v1.2.3