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

Miscellaneous cleanup in and around llmediadataclient. 

Added tags to some media-related logging in LLVOVolume.

Made LLMediaDataClient::Responder do most of its work in tick() instead of its destructor.

Added a comment to llmediadataclient.cpp that explains the idea behind the two-queue system.

Made LLMediaDataClient::sortQueue() remove requests from the queue that hold references to dead items.  This should make teleporting away solve many of the pathological queueing cases.

Updated llmediadataclient test cases to reflect the change in behavior in sortQueue().

Removed some unnecessary const-ness in LLMediaDataClient::enqueue, which caused it to have to use const_cast.
master
Monroe Linden 2010-07-21 17:46:17 -07:00
parent 76a9c1214f
commit 51f310e4ae
4 changed files with 111 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
indra/newview/llmediadataclient.cpp
View File

@ -58,6 +58,32 @@
// - Any request that gets a 503 still goes through the retry logic
//
/***************************************************************************************************************
What's up with this queueing code?
First, a bit of background:
Media on a prim was added into the system in the Viewer 2.0 timeframe. In order to avoid changing the
network format of objects, an unused field in the object (the "MediaURL" string) was repurposed to
indicate that the object had media data, and also hold a sequence number and the UUID of the agent
who last updated the data. The actual media data for objects is accessed via the "ObjectMedia" capability.
Due to concerns about sim performance, requests to this capability are rate-limited to 5 requests every
5 seconds per agent.
The initial implementation of LLMediaDataClient used a single queue to manage requests to the "ObjectMedia" cap.
Requests to the cap were queued so that objects closer to the avatar were loaded in first, since they were most
likely to be the ones the media performance manager would load.
This worked in some cases, but we found that it was possible for a scripted object that constantly updated its
media data to starve other objects, since the same queue contained both requests to load previously unseen media
data and requests to fetch media data in response to object updates.
The solution for this we came up with was to have two queues. The sorted queue contains requests to fetch media
data for objects that don't have it yet, and the round-robin queue contains requests to update media data for
objects that have already completed their initial load. When both queues are non-empty, the code ping-pongs
between them so that updates can't completely block initial load-in.
**************************************************************************************************************/
//
// Forward decls
//
@ -149,7 +175,7 @@ void LLMediaDataClient::request(const LLMediaDataClientObject::ptr_t &object, co
enqueue(new Request(getCapabilityName(), payload, object, this));
}
void LLMediaDataClient::enqueue(const Request *request)
void LLMediaDataClient::enqueue(Request *request)
{
if (request->isNew())
{
@ -161,8 +187,7 @@ void LLMediaDataClient::enqueue(const Request *request)
LL_DEBUGS("LLMediaDataClient") << "Queuing SORTED request for " << *request << LL_ENDL;
// Sadly, we have to const-cast because items put into the queue are not const
mSortedQueue.push_back(const_cast<LLMediaDataClient::Request*>(request));
mSortedQueue.push_back(request);
LL_DEBUGS("LLMediaDataClientQueue") << "SORTED queue:" << mSortedQueue << LL_ENDL;
}
@ -184,8 +209,7 @@ void LLMediaDataClient::enqueue(const Request *request)
{
LL_DEBUGS("LLMediaDataClient") << "Queuing RR request for " << *request << LL_ENDL;
// Push the request on the pending queue
// Sadly, we have to const-cast because items put into the queue are not const
mRoundRobinQueue.push_front(const_cast<LLMediaDataClient::Request*>(request));
mRoundRobinQueue.push_front(request);
LL_DEBUGS("LLMediaDataClientQueue") << "RR queue:" << mRoundRobinQueue << LL_ENDL;
}
@ -240,17 +264,64 @@ bool LLMediaDataClient::processQueueTimer()
return isEmpty();
}
bool LLMediaDataClient::request_needs_purge(LLMediaDataClient::request_ptr_t request)
{
// Check for conditions that should cause a request to get removed from the queue
if(request.isNull())
{
LL_WARNS("LLMediaDataClient") << "removing NULL request" << LL_ENDL;
return true;
}
// For some reason, the existing code put sent items onto the back of the round-robin queue and let them come through again
// before stripping them off the front. Was there a good reason for this?
// if(request->isMarkedSent())
// {
// LL_INFOS("LLMediaDataClient") << "removing : " << *request << " request is marked sent" << LL_ENDL;
// return true;
// }
const LLMediaDataClientObject *object = request->getObject();
if(object == NULL)
{
LL_INFOS("LLMediaDataClient") << "removing : " << *request << " object is NULL" << LL_ENDL;
return true;
}
if(object->isDead())
{
LL_INFOS("LLMediaDataClient") << "removing : " << *request << " object is dead" << LL_ENDL;
return true;
}
if(!object->hasMedia())
{
LL_INFOS("LLMediaDataClient") << "removing : " << *request << " object has no media" << LL_ENDL;
return true;
}
return false;
}
void LLMediaDataClient::sortQueue()
{
if(!mSortedQueue.empty())
{
// Score all items first
// Cull dead objects and score all remaining items first
request_queue_t::iterator iter = mSortedQueue.begin();
request_queue_t::iterator end = mSortedQueue.end();
while (iter != end)
{
(*iter)->updateScore();
iter++;
if(request_needs_purge(*iter))
{
iter = mSortedQueue.erase(iter);
}
else
{
(*iter)->updateScore();
iter++;
}
}
// Re-sort the list...
@ -270,6 +341,19 @@ void LLMediaDataClient::sortQueue()
}
}
}
// Cull dead items from the round robin queue as well.
for(request_queue_t::iterator iter = mRoundRobinQueue.begin(); iter != mRoundRobinQueue.end();)
{
if(request_needs_purge(*iter))
{
iter = mRoundRobinQueue.erase(iter);
}
else
{
iter++;
}
}
}
// static
@ -298,26 +382,14 @@ void LLMediaDataClient::serviceQueue()
llassert(!request.isNull());
const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject();
llassert(NULL != object);
// Check for conditions that would make us just pop and rapidly loop through
// the queue.
if(request.isNull() ||
request->isMarkedSent() ||
NULL == object ||
object->isDead() ||
!object->hasMedia())
if(request->isMarkedSent())
{
if (request.isNull())
{
LL_WARNS("LLMediaDataClient") << "Skipping NULL request" << LL_ENDL;
}
else {
LL_INFOS("LLMediaDataClient") << "Skipping : " << *request << " "
<< ((request->isMarkedSent()) ? " request is marked sent" :
((NULL == object) ? " object is NULL " :
((object->isDead()) ? "object is dead" :
((!object->hasMedia()) ? "object has no media!" : "BADNESS!")))) << LL_ENDL;
}
// Sent items that make it back to the head of the queue need to be skipped.
LL_INFOS("LLMediaDataClient") << "removing : " << *request << " request is marked sent" << LL_ENDL;
queue_p->pop_front();
continue; // jump back to the start of the quick retry loop
}
@ -448,24 +520,15 @@ LLMediaDataClient::Responder::RetryTimer::RetryTimer(F32 time, Responder *mdr)
{
}
// virtual
LLMediaDataClient::Responder::RetryTimer::~RetryTimer()
// virtual
BOOL LLMediaDataClient::Responder::RetryTimer::tick()
{
LL_DEBUGS("LLMediaDataClient") << "~RetryTimer" << *(mResponder->getRequest()) << LL_ENDL;
// XXX This is weird: Instead of doing the work in tick() (which re-schedules
// a timer, which might be risky), do it here, in the destructor. Yes, it is very odd.
// Instead of retrying, we just put the request back onto the queue
LL_INFOS("LLMediaDataClient") << "RetryTimer fired for: " << *(mResponder->getRequest()) << " retrying" << LL_ENDL;
mResponder->getRequest()->reEnqueue();
// Release the ref to the responder.
mResponder = NULL;
}
// virtual
BOOL LLMediaDataClient::Responder::RetryTimer::tick()
{
// Don't fire again
return TRUE;
}
@ -552,7 +615,7 @@ const char *LLMediaDataClient::Request::getTypeAsString() const
}
void LLMediaDataClient::Request::reEnqueue() const
void LLMediaDataClient::Request::reEnqueue()
{
// I sure hope this doesn't deref a bad pointer:
mMDC->enqueue(this);

8
indra/newview/llmediadataclient.h
View File

@ -142,7 +142,7 @@ protected:
const char *getTypeAsString() const;
// Re-enqueue thyself
void reEnqueue() const;
void reEnqueue();
F32 getRetryTimerDelay() const;
U32 getMaxNumRetries() const;
@ -185,7 +185,7 @@ protected:
//If we get back a normal response, handle it here. Default just logs it.
virtual void result(const LLSD& content);
const request_ptr_t &getRequest() const { return mRequest; }
request_ptr_t &getRequest() { return mRequest; }
protected:
virtual ~Responder();
@ -196,7 +196,6 @@ protected:
{
public:
RetryTimer(F32 time, Responder *);
virtual ~RetryTimer();
virtual BOOL tick();
private:
// back-pointer
@ -214,13 +213,14 @@ protected:
// Subclasses must override to return a cap name
virtual const char *getCapabilityName() const = 0;
virtual bool request_needs_purge(request_ptr_t request);
virtual void sortQueue();
virtual void serviceQueue();
private:
typedef std::list<request_ptr_t> request_queue_t;
void enqueue(const Request*);
void enqueue(Request*);
// Return whether the given object is/was in the queue
static LLMediaDataClient::request_ptr_t findOrRemove(request_queue_t &queue, const LLMediaDataClientObject::ptr_t &obj, bool remove, Request::Type type);

6
indra/newview/llvovolume.cpp
View File

@ -2025,12 +2025,12 @@ void LLVOVolume::mediaNavigated(LLViewerMediaImpl *impl, LLPluginClassMedia* plu
}
else
{
llwarns << "Couldn't find media entry!" << llendl;
LL_WARNS("MediaOnAPrim") << "Couldn't find media entry!" << LL_ENDL;
}
if(block_navigation)
{
llinfos << "blocking navigate to URI " << new_location << llendl;
LL_INFOS("MediaOnAPrim") << "blocking navigate to URI " << new_location << LL_ENDL;
// "bounce back" to the current URL from the media entry
mediaNavigateBounceBack(face_index);
@ -2038,7 +2038,7 @@ void LLVOVolume::mediaNavigated(LLViewerMediaImpl *impl, LLPluginClassMedia* plu
else if (sObjectMediaNavigateClient)
{
llinfos << "broadcasting navigate with URI " << new_location << llendl;
LL_DEBUGS("MediaOnAPrim") << "broadcasting navigate with URI " << new_location << LL_ENDL;
sObjectMediaNavigateClient->navigate(new LLMediaDataClientObjectImpl(this, false), face_index, new_location);
}

16
indra/newview/tests/llmediadataclient_test.cpp
View File

@ -580,25 +580,17 @@ namespace tut
::pump_timers();
// The first tick should remove the first one
// The first tick should remove the second and fourth ones, and process the first one
ensure("is not in queue 1", !mdc->isInQueue(o1));
ensure("is in queue 2", mdc->isInQueue(o2));
ensure("is not in queue 2", !mdc->isInQueue(o2));
ensure("is in queue 3", mdc->isInQueue(o3));
ensure("is in queue 4", mdc->isInQueue(o4));
ensure("is not in queue 4", !mdc->isInQueue(o4));
ensure("post records", gPostRecords->size(), 1);
::pump_timers();
// The second tick should skip the second and remove the third
ensure("is not in queue 2", !mdc->isInQueue(o2));
// The second tick should process the third, emptying the queue
ensure("is not in queue 3", !mdc->isInQueue(o3));
ensure("is in queue 4", mdc->isInQueue(o4));
ensure("post records", gPostRecords->size(), 2);
::pump_timers();
// The third tick should skip the fourth one and empty the queue.
ensure("is not in queue 4", !mdc->isInQueue(o4));
ensure("post records", gPostRecords->size(), 2);
ensure("queue empty", mdc->isEmpty());