summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2019-10-25 06:55:45 -0400
committerNat Goodspeed <nat@lindenlab.com>2020-03-25 19:05:16 -0400
commit9008124d352a195616bc8e7b88b048f479cdc4c2 (patch)
tree4821982b667c2d277d09f999986a85320e039ce5
parent26c8ccfc06bc9334c9a4d0d027e83ad0b1b92a86 (diff)
DRTVWR-476: Keep coroutine-local data on toplevel()'s stack frame.
Instead of heap-allocating a CoroData instance per coroutine, storing the pointer in a ptr_map and deleting it from the ptr_map once the fiber_specific_ptr for that coroutine is cleaned up -- just declare a stack instance on the top-level stack frame, the simplest C++ lifespan management. Derive CoroData from LLInstanceTracker to detect potential name collisions and to enumerate instances. Continue registering each coroutine's CoroData instance in our fiber_specific_ptr, but use a no-op deleter function. Make ~LLCoros() directly pump the fiber scheduler a few times, instead of having a special "LLApp" listener.
-rw-r--r--indra/llcommon/llcoros.cpp97
-rw-r--r--indra/llcommon/llcoros.h13
-rw-r--r--indra/newview/llvoicevivox.cpp6
3 files changed, 45 insertions, 71 deletions
diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp
index 78a0c5d225..febe74b559 100644
--- a/indra/llcommon/llcoros.cpp
+++ b/indra/llcommon/llcoros.cpp
@@ -76,9 +76,9 @@ LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller)
// It's tempting to provide a distinct name for each thread's "main
// coroutine." But as getName() has always returned the empty string
// to mean "not in a coroutine," empty string should suffice here.
- static CoroData sMain("");
- // We need not reset() the local_ptr to this read-only data: reuse the
- // same instance for every thread's main coroutine.
+ static thread_local CoroData sMain("");
+ // We need not reset() the local_ptr to this instance; we'll simply
+ // find it again every time we discover that current is null.
current = &sMain;
}
return *current;
@@ -123,16 +123,36 @@ LLCoros::LLCoros():
// boost::context::guarded_stack_allocator::default_stacksize();
// empirically this is insufficient.
#if ADDRESS_SIZE == 64
- mStackSize(512*1024)
+ mStackSize(512*1024),
#else
- mStackSize(256*1024)
+ mStackSize(256*1024),
#endif
+ // mCurrent does NOT own the current CoroData instance -- it simply
+ // points to it. So initialize it with a no-op deleter.
+ mCurrent{ [](CoroData*){} }
{
}
LLCoros::~LLCoros()
{
- printActiveCoroutines("at LLCoros destruction");
+ printActiveCoroutines("at entry to ~LLCoros()");
+ // Other LLApp status-change listeners do things like close
+ // work queues and inject the Stop exception into pending
+ // promises, to force coroutines waiting on those things to
+ // notice and terminate. The only problem is that by the time
+ // LLApp sets "quitting" status, the main loop has stopped
+ // pumping the fiber scheduler with yield() calls. A waiting
+ // coroutine still might not wake up until after resources on
+ // which it depends have been freed. Pump it a few times
+ // ourselves. Of course, stop pumping as soon as the last of
+ // the coroutines has terminated.
+ for (size_t count = 0; count < 10 && CoroData::instanceCount() > 0; ++count)
+ {
+ // don't use llcoro::suspend() because that module depends
+ // on this one
+ boost::this_fiber::yield();
+ }
+ printActiveCoroutines("after pumping");
}
std::string LLCoros::generateDistinctName(const std::string& prefix) const
@@ -149,7 +169,7 @@ std::string LLCoros::generateDistinctName(const std::string& prefix) const
std::string name(prefix);
// Until we find an unused name, append a numeric suffix for uniqueness.
- while (mCoros.find(name) != mCoros.end())
+ while (CoroData::getInstance(name))
{
name = STRINGIZE(prefix << unique++);
}
@@ -186,16 +206,17 @@ void LLCoros::setStackSize(S32 stacksize)
void LLCoros::printActiveCoroutines(const std::string& when)
{
LL_INFOS("LLCoros") << "Number of active coroutines " << when
- << ": " << (S32)mCoros.size() << LL_ENDL;
- if (mCoros.size() > 0)
+ << ": " << CoroData::instanceCount() << LL_ENDL;
+ if (CoroData::instanceCount() > 0)
{
LL_INFOS("LLCoros") << "-------------- List of active coroutines ------------";
F64 time = LLTimer::getTotalSeconds();
- for (const auto& pair : mCoros)
+ for (auto it(CoroData::beginInstances()), end(CoroData::endInstances());
+ it != end; ++it)
{
- F64 life_time = time - pair.second->mCreationTime;
- LL_CONT << LL_NEWLINE << pair.first << ' ' << pair.second->mStatus
- << " life: " << life_time;
+ F64 life_time = time - it->mCreationTime;
+ LL_CONT << LL_NEWLINE
+ << it->mName << ' ' << it->mStatus << " life: " << life_time;
}
LL_CONT << LL_ENDL;
LL_INFOS("LLCoros") << "-----------------------------------------------------" << LL_ENDL;
@@ -265,17 +286,10 @@ void LLCoros::winlevel(const callable_t& callable)
// case, we WANT to copy both the name and the callable to our local stack!
void LLCoros::toplevel(std::string name, callable_t callable)
{
- CoroData* corodata = new CoroData(name);
- if (corodata == NULL)
- {
- // Out of memory?
- printActiveCoroutines();
- LL_ERRS("LLCoros") << "Failed to start coroutine: " << name << " Stacksize: " << mStackSize << " Total coroutines: " << mCoros.size() << LL_ENDL;
- }
- // Store it in our pointer map.
- mCoros.insert(name, corodata);
- // also set it as current
- mCurrent.reset(corodata);
+ // keep the CoroData on this top-level function's stack frame
+ CoroData corodata(name);
+ // set it as current
+ mCurrent.reset(&corodata);
// run the code the caller actually wants in the coroutine
try
@@ -323,43 +337,10 @@ void LLCoros::checkStop()
}
LLCoros::CoroData::CoroData(const std::string& name):
+ LLInstanceTracker<CoroData, std::string>(name),
mName(name),
// don't consume events unless specifically directed
mConsuming(false),
mCreationTime(LLTimer::getTotalSeconds())
{
}
-
-void LLCoros::delete_CoroData(CoroData* cdptr)
-{
- // This custom cleanup function is necessarily static. Find and bind the
- // LLCoros instance.
- // In case the LLCoros instance has already been deleted, just warn and
- // scoot. We do NOT want to reinstantiate LLCoros during shutdown!
- if (wasDeleted())
- {
- // The LLSingletons involved in logging may have been deleted too.
- // This warning may help developers track down coroutines that have
- // not yet been cleaned up.
- // But cdptr is very likely a dangling pointer by this time, so don't
- // try to dereference mName.
- logwarns("Coroutine terminating after LLCoros instance deleted");
- return;
- }
-
- LLCoros& self(LLCoros::instance());
- // We set mCurrent on entry to a new fiber, expecting that the
- // corresponding entry has already been stored in mCoros. It is an
- // error if we do not find that entry.
- CoroMap::iterator found = self.mCoros.find(cdptr->mName);
- if (found == self.mCoros.end())
- {
- LL_ERRS("LLCoros") << "Coroutine '" << cdptr->mName << "' terminated "
- << "without being stored in LLCoros::mCoros"
- << LL_ENDL;
- }
-
- // Oh good, we found the mCoros entry. Erase it. Because it's a ptr_map,
- // that will implicitly delete this CoroData.
- self.mCoros.erase(found);
-}
diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index de7b691284..7b3420cc8f 100644
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -34,7 +34,7 @@
#include <boost/fiber/future/promise.hpp>
#include <boost/fiber/future/future.hpp>
#include "llsingleton.h"
-#include <boost/ptr_container/ptr_map.hpp>
+#include "llinstancetracker.h"
#include <boost/function.hpp>
#include <string>
@@ -269,7 +269,7 @@ private:
S32 mStackSize;
// coroutine-local storage, as it were: one per coro we track
- struct CoroData
+ struct CoroData: public LLInstanceTracker<CoroData, std::string>
{
CoroData(const std::string& name);
@@ -281,18 +281,11 @@ private:
std::string mStatus;
F64 mCreationTime; // since epoch
};
- typedef boost::ptr_map<std::string, CoroData> CoroMap;
- CoroMap mCoros;
// Identify the current coroutine's CoroData. This local_ptr isn't static
// because it's a member of an LLSingleton, and we rely on it being
// cleaned up in proper dependency order.
- // As each coroutine terminates, use our custom cleanup function to remove
- // the corresponding entry from mCoros.
- local_ptr<CoroData> mCurrent{delete_CoroData};
-
- // Cleanup function for each fiber's instance of mCurrent.
- static void delete_CoroData(CoroData* cdptr);
+ local_ptr<CoroData> mCurrent;
};
namespace llcoro
diff --git a/indra/newview/llvoicevivox.cpp b/indra/newview/llvoicevivox.cpp
index 1141b29163..23214a6998 100644
--- a/indra/newview/llvoicevivox.cpp
+++ b/indra/newview/llvoicevivox.cpp
@@ -390,7 +390,7 @@ void LLVivoxVoiceClient::init(LLPumpIO *pump)
// constructor will set up LLVoiceClient::getInstance()
LLVivoxVoiceClient::getInstance()->mPump = pump;
-// LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();",
+// LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro",
// boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance()));
}
@@ -2537,7 +2537,7 @@ void LLVivoxVoiceClient::tuningStart()
mTuningMode = true;
if (!mIsCoroutineActive)
{
- LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();",
+ LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro",
boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance()));
}
else if (mIsInChannel)
@@ -5132,7 +5132,7 @@ void LLVivoxVoiceClient::setVoiceEnabled(bool enabled)
if (!mIsCoroutineActive)
{
- LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro();",
+ LLCoros::instance().launch("LLVivoxVoiceClient::voiceControlCoro",
boost::bind(&LLVivoxVoiceClient::voiceControlCoro, LLVivoxVoiceClient::getInstance()));
}
else