From ba3f7965e35e4eb0d7fcecc7267ec3af5f2d8d87 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Thu, 19 Nov 2009 16:35:11 -0800 Subject: Made LLMediaDataClient not send requests on behalf of objects that are marked as dead. When LLMediaDataClient::QueueTimer::tick() encounters an object at the head of the queue that's dead, it will now remove that object and loop, instead of sending a request and waiting for the tick timer to fire again. Added an isDead() function to LLMediaDataClientObject, and an additional unit test that verifies the handling of dead objects. --- indra/newview/llmediadataclient.cpp | 116 +++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 48 deletions(-) (limited to 'indra/newview/llmediadataclient.cpp') diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp index 986c14acff..6666a03ee4 100755 --- a/indra/newview/llmediadataclient.cpp +++ b/indra/newview/llmediadataclient.cpp @@ -371,67 +371,87 @@ BOOL LLMediaDataClient::QueueTimer::tick() } LLMediaDataClient::PriorityQueue &queue = *(mMDC->pRequestQueue); - - if (queue.empty()) + + if(!queue.empty()) { - LL_DEBUGS("LLMediaDataClient") << "queue empty: " << queue << LL_ENDL; - return TRUE; + LL_INFOS("LLMediaDataClient") << "QueueTimer::tick() started, queue is: " << queue << LL_ENDL; } - LL_INFOS("LLMediaDataClient") << "QueueTimer::tick() started, queue is: " << queue << LL_ENDL; - - // Peel one off of the items from the queue, and execute request - request_ptr_t request = queue.top(); - llassert(!request.isNull()); - const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject(); - bool performed_request = false; - bool error = false; - llassert(NULL != object); - if (NULL != object && object->hasMedia()) + // quick retry loop for cases where we shouldn't wait for the next timer tick + while(true) { - std::string url = request->getCapability(); - if (!url.empty()) + if (queue.empty()) { - const LLSD &sd_payload = request->getPayload(); - LL_INFOS("LLMediaDataClient") << "Sending request for " << *request << LL_ENDL; - - // Call the subclass for creating the responder - LLHTTPClient::post(url, sd_payload, mMDC->createResponder(request)); - performed_request = true; + LL_DEBUGS("LLMediaDataClient") << "queue empty: " << queue << LL_ENDL; + return TRUE; } - else { - LL_INFOS("LLMediaDataClient") << "NOT Sending request for " << *request << ": empty cap url!" << LL_ENDL; - } - } - else { - if (request.isNull()) + + // Peel one off of the items from the queue, and execute request + request_ptr_t request = queue.top(); + llassert(!request.isNull()); + const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject(); + bool performed_request = false; + bool error = false; + llassert(NULL != object); + + if(object->isDead()) { - LL_WARNS("LLMediaDataClient") << "Not Sending request: NULL request!" << LL_ENDL; + // This object has been marked dead. Pop it and move on to the next item in the queue immediately. + LL_INFOS("LLMediaDataClient") << "Skipping " << *request << ": object is dead!" << LL_ENDL; + queue.pop(); + continue; // jump back to the start of the quick retry loop } - else if (NULL == object) + + if (NULL != object && object->hasMedia()) { - LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " NULL object!" << LL_ENDL; + std::string url = request->getCapability(); + if (!url.empty()) + { + const LLSD &sd_payload = request->getPayload(); + LL_INFOS("LLMediaDataClient") << "Sending request for " << *request << LL_ENDL; + + // Call the subclass for creating the responder + LLHTTPClient::post(url, sd_payload, mMDC->createResponder(request)); + performed_request = true; + } + else { + LL_INFOS("LLMediaDataClient") << "NOT Sending request for " << *request << ": empty cap url!" << LL_ENDL; + } } - else if (!object->hasMedia()) - { - LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " hasMedia() is false!" << LL_ENDL; + else { + if (request.isNull()) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request: NULL request!" << LL_ENDL; + } + else if (NULL == object) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " NULL object!" << LL_ENDL; + } + else if (!object->hasMedia()) + { + LL_WARNS("LLMediaDataClient") << "Not Sending request for " << *request << " hasMedia() is false!" << LL_ENDL; + } + error = true; } - error = true; - } - bool exceeded_retries = request->getRetryCount() > mMDC->mMaxNumRetries; - if (performed_request || exceeded_retries || error) // Try N times before giving up - { - if (exceeded_retries) + bool exceeded_retries = request->getRetryCount() > mMDC->mMaxNumRetries; + if (performed_request || exceeded_retries || error) // Try N times before giving up { - LL_WARNS("LLMediaDataClient") << "Could not send request " << *request << " for " - << mMDC->mMaxNumRetries << " tries...popping object id " << object->getID() << LL_ENDL; - // XXX Should we bring up a warning dialog?? + if (exceeded_retries) + { + LL_WARNS("LLMediaDataClient") << "Could not send request " << *request << " for " + << mMDC->mMaxNumRetries << " tries...popping object id " << object->getID() << LL_ENDL; + // XXX Should we bring up a warning dialog?? + } + queue.pop(); } - queue.pop(); - } - else { - request->incRetryCount(); - } + else { + request->incRetryCount(); + } + + // end of quick retry loop -- any cases where we want to loop will use 'continue' to jump back to the start. + break; + } + LL_DEBUGS("LLMediaDataClient") << "QueueTimer::tick() finished, queue is now: " << (*(mMDC->pRequestQueue)) << LL_ENDL; return queue.empty(); -- cgit v1.2.3 From 3bb86f91bc345f74057debe1f8f680098895a40e Mon Sep 17 00:00:00 2001 From: Rick Pasetto Date: Fri, 20 Nov 2009 14:01:49 -0800 Subject: Clean up some logging --- indra/newview/llmediadataclient.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra/newview/llmediadataclient.cpp') diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp index 6666a03ee4..badef4c7ae 100755 --- a/indra/newview/llmediadataclient.cpp +++ b/indra/newview/llmediadataclient.cpp @@ -637,6 +637,9 @@ void LLObjectMediaNavigateClient::navigate(LLMediaDataClientObject *object, U8 t sd_payload[LLTextureEntry::OBJECT_ID_KEY] = object->getID(); sd_payload[LLMediaEntry::CURRENT_URL_KEY] = url; sd_payload[LLTextureEntry::TEXTURE_INDEX_KEY] = (LLSD::Integer)texture_index; + + LL_INFOS("LLMediaDataClient") << "navigate() initiated: " << ll_print_sd(sd_payload) << LL_ENDL; + request(object, sd_payload); } -- cgit v1.2.3 From f276b73d974a4b47d50e71922846efe9ee0b1409 Mon Sep 17 00:00:00 2001 From: Rick Pasetto Date: Tue, 24 Nov 2009 14:26:00 -0800 Subject: DEV-41998 - refactor mediadataclient to use a std::list, and re-sort every time an item is pulled off the queue Review #43 This change refactors mediadataclient to no longer use a PriorityQueue (which sorts only on insertion), but rather just use a std::list which is re-sorted on insert, and also when "popped" (at the time the queue timer goes off). Also implemented a unit test to make sure re-sorting occurs on timer tick. --- indra/newview/llmediadataclient.cpp | 60 ++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) (limited to 'indra/newview/llmediadataclient.cpp') diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp index badef4c7ae..924f5a2598 100755 --- a/indra/newview/llmediadataclient.cpp +++ b/indra/newview/llmediadataclient.cpp @@ -260,7 +260,8 @@ void LLMediaDataClient::Responder::result(const LLSD& content) // ////////////////////////////////////////////////////////////////////////////////////// -bool LLMediaDataClient::Comparator::operator() (const request_ptr_t &o1, const request_ptr_t &o2) const +// static +bool LLMediaDataClient::compareRequests(const request_ptr_t &o1, const request_ptr_t &o2) { if (o2.isNull()) return true; if (o1.isNull()) return false; @@ -277,20 +278,13 @@ bool LLMediaDataClient::Comparator::operator() (const request_ptr_t &o1, const r // 3: One item with an impl, another without: item with impl wins // (XXX is that what we want?) // Calculate the scores for each. - F64 o1_score = Comparator::getObjectScore(o1->getObject()); - F64 o2_score = Comparator::getObjectScore(o2->getObject()); - - // XXX Weird: a higher score should go earlier, but by observation I notice - // that this causes further-away objects load first. This is counterintuitive - // to the priority_queue Comparator, which states that this function should - // return 'true' if o1 should be *before* o2. - // In other words, I'd have expected that the following should return - // ( o1_score > o2_score). - return ( o1_score < o2_score ); + F64 o1_score = getObjectScore(o1->getObject()); + F64 o2_score = getObjectScore(o2->getObject()); + return ( o1_score > o2_score ); } - + // static -F64 LLMediaDataClient::Comparator::getObjectScore(const LLMediaDataClientObject::ptr_t &obj) +F64 LLMediaDataClient::getObjectScore(const LLMediaDataClientObject::ptr_t &obj) { // *TODO: make this less expensive? F64 dist = obj->getDistanceFromAvatar() + 0.1; // avoids div by 0 @@ -310,12 +304,12 @@ F64 LLMediaDataClient::Comparator::getObjectScore(const LLMediaDataClientObject: ////////////////////////////////////////////////////////////////////////////////////// // dump the queue -std::ostream& operator<<(std::ostream &s, const LLMediaDataClient::PriorityQueue &q) +std::ostream& operator<<(std::ostream &s, const LLMediaDataClient::request_queue_t &q) { int i = 0; - std::vector::const_iterator iter = q.c.begin(); - std::vector::const_iterator end = q.c.end(); - while (iter < end) + LLMediaDataClient::request_queue_t::const_iterator iter = q.begin(); + LLMediaDataClient::request_queue_t::const_iterator end = q.end(); + while (iter != end) { s << "\t" << i << "]: " << (*iter)->getObject()->getID().asString(); iter++; @@ -325,11 +319,11 @@ std::ostream& operator<<(std::ostream &s, const LLMediaDataClient::PriorityQueue } // find the given object in the queue. -bool LLMediaDataClient::PriorityQueue::find(const LLMediaDataClientObject::ptr_t &obj) const +bool LLMediaDataClient::find(const LLMediaDataClientObject::ptr_t &obj) const { - std::vector::const_iterator iter = c.begin(); - std::vector::const_iterator end = c.end(); - while (iter < end) + request_queue_t::const_iterator iter = pRequestQueue->begin(); + request_queue_t::const_iterator end = pRequestQueue->end(); + while (iter != end) { if (obj->getID() == (*iter)->getObject()->getID()) { @@ -370,13 +364,17 @@ BOOL LLMediaDataClient::QueueTimer::tick() return TRUE; } - LLMediaDataClient::PriorityQueue &queue = *(mMDC->pRequestQueue); + request_queue_t &queue = *(mMDC->pRequestQueue); if(!queue.empty()) { LL_INFOS("LLMediaDataClient") << "QueueTimer::tick() started, queue is: " << queue << LL_ENDL; - } + // Re-sort the list every time... + // XXX Is this really what we want? + queue.sort(LLMediaDataClient::compareRequests); + } + // quick retry loop for cases where we shouldn't wait for the next timer tick while(true) { @@ -387,7 +385,7 @@ BOOL LLMediaDataClient::QueueTimer::tick() } // Peel one off of the items from the queue, and execute request - request_ptr_t request = queue.top(); + request_ptr_t request = queue.front(); llassert(!request.isNull()); const LLMediaDataClientObject *object = (request.isNull()) ? NULL : request->getObject(); bool performed_request = false; @@ -398,7 +396,7 @@ BOOL LLMediaDataClient::QueueTimer::tick() { // This object has been marked dead. Pop it and move on to the next item in the queue immediately. LL_INFOS("LLMediaDataClient") << "Skipping " << *request << ": object is dead!" << LL_ENDL; - queue.pop(); + queue.pop_front(); continue; // jump back to the start of the quick retry loop } @@ -442,7 +440,7 @@ BOOL LLMediaDataClient::QueueTimer::tick() << mMDC->mMaxNumRetries << " tries...popping object id " << object->getID() << LL_ENDL; // XXX Should we bring up a warning dialog?? } - queue.pop(); + queue.pop_front(); } else { request->incRetryCount(); @@ -451,7 +449,7 @@ BOOL LLMediaDataClient::QueueTimer::tick() // end of quick retry loop -- any cases where we want to loop will use 'continue' to jump back to the start. break; } - + LL_DEBUGS("LLMediaDataClient") << "QueueTimer::tick() finished, queue is now: " << (*(mMDC->pRequestQueue)) << LL_ENDL; return queue.empty(); @@ -488,7 +486,9 @@ void LLMediaDataClient::enqueue(const Request *request) LL_INFOS("LLMediaDataClient") << "Queuing request for " << *request << LL_ENDL; // Push the request on the priority queue // Sadly, we have to const-cast because items put into the queue are not const - pRequestQueue->push(const_cast(request)); + pRequestQueue->push_back(const_cast(request)); + // sort the list + pRequestQueue->sort(LLMediaDataClient::compareRequests); LL_DEBUGS("LLMediaDataClient") << "Queue:" << (*pRequestQueue) << LL_ENDL; // Start the timer if not already running startQueueTimer(); @@ -508,7 +508,7 @@ LLMediaDataClient::LLMediaDataClient(F32 queue_timer_delay, mMaxNumRetries(max_retries), mQueueTimerIsRunning(false) { - pRequestQueue = new PriorityQueue(); + pRequestQueue = new request_queue_t(); } LLMediaDataClient::~LLMediaDataClient() @@ -529,7 +529,7 @@ bool LLMediaDataClient::isEmpty() const bool LLMediaDataClient::isInQueue(const LLMediaDataClientObject::ptr_t &object) const { - return (NULL == pRequestQueue) ? false : pRequestQueue->find(object); + return (NULL == pRequestQueue) ? false : find(object); } ////////////////////////////////////////////////////////////////////////////////////// -- cgit v1.2.3 From c8519c5a7b41652499ead70736f0d61470ee6ce3 Mon Sep 17 00:00:00 2001 From: Rick Pasetto Date: Tue, 24 Nov 2009 15:55:53 -0800 Subject: Code review feedback: Don't re-sort when adding to the queue: it isn't really necessary since we sort every time we pull off the queue. --- indra/newview/llmediadataclient.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'indra/newview/llmediadataclient.cpp') diff --git a/indra/newview/llmediadataclient.cpp b/indra/newview/llmediadataclient.cpp index 924f5a2598..3c337961e1 100755 --- a/indra/newview/llmediadataclient.cpp +++ b/indra/newview/llmediadataclient.cpp @@ -487,8 +487,6 @@ void LLMediaDataClient::enqueue(const Request *request) // Push the request on the priority queue // Sadly, we have to const-cast because items put into the queue are not const pRequestQueue->push_back(const_cast(request)); - // sort the list - pRequestQueue->sort(LLMediaDataClient::compareRequests); LL_DEBUGS("LLMediaDataClient") << "Queue:" << (*pRequestQueue) << LL_ENDL; // Start the timer if not already running startQueueTimer(); -- cgit v1.2.3