From 8f4f7452308d41467b021ae0da821b33f559dd79 Mon Sep 17 00:00:00 2001 From: "Brad Payne (Vir Linden)" Date: Wed, 3 Jul 2013 12:45:56 -0400 Subject: SH-4226 WIP - try to be smarter about when to send appearance update requests, removed many redundant calls --- indra/newview/llappearancemgr.cpp | 413 +++++++++++++++++++++--------------- indra/newview/llappearancemgr.h | 5 +- indra/newview/llhttpretrypolicy.cpp | 5 + indra/newview/llhttpretrypolicy.h | 4 + indra/newview/llvoavatar.cpp | 15 +- 5 files changed, 270 insertions(+), 172 deletions(-) diff --git a/indra/newview/llappearancemgr.cpp b/indra/newview/llappearancemgr.cpp index 12c8c82af4..485c47b2f7 100755 --- a/indra/newview/llappearancemgr.cpp +++ b/indra/newview/llappearancemgr.cpp @@ -3037,158 +3037,274 @@ void LLAppearanceMgr::updateClothingOrderingInfo(LLUUID cat_id, class RequestAgentUpdateAppearanceResponder: public LLHTTPClient::Responder { LOG_CLASS(RequestAgentUpdateAppearanceResponder); + + friend class LLAppearanceMgr; + public: - RequestAgentUpdateAppearanceResponder() - { - bool retry_on_4xx = true; - mRetryPolicy = new LLAdaptiveRetryPolicy(1.0, 32.0, 2.0, 10, retry_on_4xx); - } + RequestAgentUpdateAppearanceResponder(); - virtual ~RequestAgentUpdateAppearanceResponder() - { - } + virtual ~RequestAgentUpdateAppearanceResponder(); + +private: + // Called when sendServerAppearanceUpdate called. May or may not + // trigger a request depending on various bits of state. + void onRequestRequested(); + + // Post the actual appearance request to cap. + void sendRequest(); + + void debugCOF(const LLSD& content); protected: // Successful completion. - /* virtual */ void httpSuccess() - { - const LLSD& content = getContent(); - if (!content.isMap()) - { - failureResult(HTTP_INTERNAL_ERROR, "Malformed response contents", content); - return; - } - if (content["success"].asBoolean()) - { - //LL_DEBUGS("Avatar") << dumpResponse() << LL_ENDL; - if (gSavedSettings.getBOOL("DebugAvatarAppearanceMessage")) - { - dump_sequential_xml(gAgentAvatarp->getFullname() + "_appearance_request_ok", content); - } - } - else - { - failureResult(HTTP_INTERNAL_ERROR, "Non-success response", content); - } - } + /* virtual */ void httpSuccess(); // Error - /*virtual*/ void httpFailure() + /*virtual*/ void httpFailure(); + + void onFailure(); + void onSuccess(); + + S32 mInFlightCounter; + LLTimer mInFlightTimer; + LLPointer mRetryPolicy; +}; + +RequestAgentUpdateAppearanceResponder::RequestAgentUpdateAppearanceResponder() +{ + bool retry_on_4xx = true; + mRetryPolicy = new LLAdaptiveRetryPolicy(1.0, 32.0, 2.0, 10, retry_on_4xx); + mInFlightCounter = 0; +} + +RequestAgentUpdateAppearanceResponder::~RequestAgentUpdateAppearanceResponder() +{ +} + +void RequestAgentUpdateAppearanceResponder::onRequestRequested() +{ + // If we have already received an update for this or higher cof version, ignore. + S32 cof_version = LLAppearanceMgr::instance().getCOFVersion(); + S32 last_rcv = gAgentAvatarp->mLastUpdateReceivedCOFVersion; + S32 last_req = gAgentAvatarp->mLastUpdateRequestCOFVersion; + LL_DEBUGS("Avatar") << "cof_version " << cof_version + << " last_rcv " << last_rcv + << " last_req " << last_req + << " in flight " << mInFlightCounter << llendl; + if ((mInFlightCounter>0) && (mInFlightTimer.hasExpired())) + { + LL_WARNS("Avatar") << "in flight timer expired, resetting " << llendl; + mInFlightCounter = 0; + } + if (cof_version < last_rcv) { - LL_WARNS("Avatar") << "appearance update request failed, status " - << getStatus() << " reason " << getReason() << LL_ENDL; + LL_DEBUGS("Avatar") << "Have already received update for cof version " << last_rcv + << " will not request for " << cof_version << llendl; + return; + } + if (mInFlightCounter>0 && last_req >= cof_version) + { + LL_DEBUGS("Avatar") << "Request already in flight for cof version " << last_req + << " will not request for " << cof_version << llendl; + return; + } - if (gSavedSettings.getBOOL("DebugAvatarAppearanceMessage")) - { - const LLSD& content = getContent(); - dump_sequential_xml(gAgentAvatarp->getFullname() + "_appearance_request_error", content); - debugCOF(content); - } - onFailure(); + // Actually send the request. + LL_DEBUGS("Avatar") << "Will send request for cof_version " << cof_version << llendl; + mRetryPolicy->reset(); + sendRequest(); +} + +void RequestAgentUpdateAppearanceResponder::sendRequest() +{ + if (gAgentAvatarp->isEditingAppearance()) + { + // don't send out appearance updates if in appearance editing mode + return; } - void onFailure() + if (!gAgent.getRegion()) { - F32 seconds_to_wait; - mRetryPolicy->onFailure(getStatus(), getResponseHeaders()); - if (mRetryPolicy->shouldRetry(seconds_to_wait)) - { - llinfos << "retrying" << llendl; - doAfterInterval(boost::bind(&LLAppearanceMgr::requestServerAppearanceUpdate, - LLAppearanceMgr::getInstance(), - LLHTTPClient::ResponderPtr(this)), - seconds_to_wait); - } - else + llwarns << "Region not set, cannot request server appearance update" << llendl; + return; + } + if (gAgent.getRegion()->getCentralBakeVersion()==0) + { + llwarns << "Region does not support baking" << llendl; + } + std::string url = gAgent.getRegion()->getCapability("UpdateAvatarAppearance"); + if (url.empty()) + { + llwarns << "No cap for UpdateAvatarAppearance." << llendl; + return; + } + + LLSD body; + S32 cof_version = LLAppearanceMgr::instance().getCOFVersion(); + if (gSavedSettings.getBOOL("DebugAvatarExperimentalServerAppearanceUpdate")) + { + body = LLAppearanceMgr::instance().dumpCOF(); + } + else + { + body["cof_version"] = cof_version; + if (gSavedSettings.getBOOL("DebugForceAppearanceRequestFailure")) { - llwarns << "giving up after too many retries" << llendl; + body["cof_version"] = cof_version+999; } - } + } + LL_DEBUGS("Avatar") << "request url " << url << " my_cof_version " << cof_version << llendl; + + mInFlightCounter++; + mInFlightTimer.setTimerExpirySec(30.0); + LLHTTPClient::post(url, body, this); + llassert(cof_version >= gAgentAvatarp->mLastUpdateRequestCOFVersion); + gAgentAvatarp->mLastUpdateRequestCOFVersion = cof_version; +} - void debugCOF(const LLSD& content) +void RequestAgentUpdateAppearanceResponder::debugCOF(const LLSD& content) +{ + LL_INFOS("Avatar") << "AIS COF, version received: " << content["expected"].asInteger() + << " ================================= " << llendl; + std::set ais_items, local_items; + const LLSD& cof_raw = content["cof_raw"]; + for (LLSD::array_const_iterator it = cof_raw.beginArray(); + it != cof_raw.endArray(); ++it) { - LL_INFOS("Avatar") << "AIS COF, version received: " << content["expected"].asInteger() - << " ================================= " << llendl; - std::set ais_items, local_items; - const LLSD& cof_raw = content["cof_raw"]; - for (LLSD::array_const_iterator it = cof_raw.beginArray(); - it != cof_raw.endArray(); ++it) + const LLSD& item = *it; + if (item["parent_id"].asUUID() == LLAppearanceMgr::instance().getCOF()) { - const LLSD& item = *it; - if (item["parent_id"].asUUID() == LLAppearanceMgr::instance().getCOF()) + ais_items.insert(item["item_id"].asUUID()); + if (item["type"].asInteger() == 24) // link { - ais_items.insert(item["item_id"].asUUID()); - if (item["type"].asInteger() == 24) // link - { - LL_INFOS("Avatar") << "AIS Link: item_id: " << item["item_id"].asUUID() - << " linked_item_id: " << item["asset_id"].asUUID() - << " name: " << item["name"].asString() - << llendl; - } - else if (item["type"].asInteger() == 25) // folder link - { - LL_INFOS("Avatar") << "AIS Folder link: item_id: " << item["item_id"].asUUID() - << " linked_item_id: " << item["asset_id"].asUUID() - << " name: " << item["name"].asString() - << llendl; + LL_INFOS("Avatar") << "AIS Link: item_id: " << item["item_id"].asUUID() + << " linked_item_id: " << item["asset_id"].asUUID() + << " name: " << item["name"].asString() + << llendl; + } + else if (item["type"].asInteger() == 25) // folder link + { + LL_INFOS("Avatar") << "AIS Folder link: item_id: " << item["item_id"].asUUID() + << " linked_item_id: " << item["asset_id"].asUUID() + << " name: " << item["name"].asString() + << llendl; - } - else - { - LL_INFOS("Avatar") << "AIS Other: item_id: " << item["item_id"].asUUID() - << " linked_item_id: " << item["asset_id"].asUUID() - << " name: " << item["name"].asString() - << " type: " << item["type"].asInteger() - << llendl; - } } - } - LL_INFOS("Avatar") << llendl; - LL_INFOS("Avatar") << "Local COF, version requested: " << content["observed"].asInteger() - << " ================================= " << llendl; - LLInventoryModel::cat_array_t cat_array; - LLInventoryModel::item_array_t item_array; - gInventory.collectDescendents(LLAppearanceMgr::instance().getCOF(), - cat_array,item_array,LLInventoryModel::EXCLUDE_TRASH); - for (S32 i=0; igetUUID()); - LL_INFOS("Avatar") << "LOCAL: item_id: " << inv_item->getUUID() - << " linked_item_id: " << inv_item->getLinkedUUID() - << " name: " << inv_item->getName() - << " parent: " << inv_item->getParentUUID() - << llendl; - } - LL_INFOS("Avatar") << " ================================= " << llendl; - S32 local_only = 0, ais_only = 0; - for (std::set::iterator it = local_items.begin(); it != local_items.end(); ++it) - { - if (ais_items.find(*it) == ais_items.end()) + else { - LL_INFOS("Avatar") << "LOCAL ONLY: " << *it << llendl; - local_only++; + LL_INFOS("Avatar") << "AIS Other: item_id: " << item["item_id"].asUUID() + << " linked_item_id: " << item["asset_id"].asUUID() + << " name: " << item["name"].asString() + << " type: " << item["type"].asInteger() + << llendl; } } - for (std::set::iterator it = ais_items.begin(); it != ais_items.end(); ++it) + } + LL_INFOS("Avatar") << llendl; + LL_INFOS("Avatar") << "Local COF, version requested: " << content["observed"].asInteger() + << " ================================= " << llendl; + LLInventoryModel::cat_array_t cat_array; + LLInventoryModel::item_array_t item_array; + gInventory.collectDescendents(LLAppearanceMgr::instance().getCOF(), + cat_array,item_array,LLInventoryModel::EXCLUDE_TRASH); + for (S32 i=0; igetUUID()); + LL_INFOS("Avatar") << "LOCAL: item_id: " << inv_item->getUUID() + << " linked_item_id: " << inv_item->getLinkedUUID() + << " name: " << inv_item->getName() + << " parent: " << inv_item->getParentUUID() + << llendl; + } + LL_INFOS("Avatar") << " ================================= " << llendl; + S32 local_only = 0, ais_only = 0; + for (std::set::iterator it = local_items.begin(); it != local_items.end(); ++it) + { + if (ais_items.find(*it) == ais_items.end()) { - if (local_items.find(*it) == local_items.end()) - { - LL_INFOS("Avatar") << "AIS ONLY: " << *it << llendl; - ais_only++; - } + LL_INFOS("Avatar") << "LOCAL ONLY: " << *it << llendl; + local_only++; } - if (local_only==0 && ais_only==0) + } + for (std::set::iterator it = ais_items.begin(); it != ais_items.end(); ++it) + { + if (local_items.find(*it) == local_items.end()) { - LL_INFOS("Avatar") << "COF contents identical, only version numbers differ (req " - << content["observed"].asInteger() - << " rcv " << content["expected"].asInteger() - << ")" << llendl; + LL_INFOS("Avatar") << "AIS ONLY: " << *it << llendl; + ais_only++; } } + if (local_only==0 && ais_only==0) + { + LL_INFOS("Avatar") << "COF contents identical, only version numbers differ (req " + << content["observed"].asInteger() + << " rcv " << content["expected"].asInteger() + << ")" << llendl; + } +} + +/* virtual */ void RequestAgentUpdateAppearanceResponder::httpSuccess() +{ + const LLSD& content = getContent(); + if (!content.isMap()) + { + failureResult(HTTP_INTERNAL_ERROR, "Malformed response contents", content); + return; + } + if (content["success"].asBoolean()) + { + //LL_DEBUGS("Avatar") << dumpResponse() << LL_ENDL; + if (gSavedSettings.getBOOL("DebugAvatarAppearanceMessage")) + { + dump_sequential_xml(gAgentAvatarp->getFullname() + "_appearance_request_ok", content); + } + + onSuccess(); + } + else + { + failureResult(HTTP_INTERNAL_ERROR, "Non-success response", content); + } +} + +void RequestAgentUpdateAppearanceResponder::onSuccess() +{ + mInFlightCounter = llmax(mInFlightCounter-1,0); +} + +/*virtual*/ void RequestAgentUpdateAppearanceResponder::httpFailure() +{ + LL_WARNS("Avatar") << "appearance update request failed, status " + << getStatus() << " reason " << getReason() << LL_ENDL; + + if (gSavedSettings.getBOOL("DebugAvatarAppearanceMessage")) + { + const LLSD& content = getContent(); + dump_sequential_xml(gAgentAvatarp->getFullname() + "_appearance_request_error", content); + debugCOF(content); + } + onFailure(); +} + +void RequestAgentUpdateAppearanceResponder::onFailure() +{ + mInFlightCounter = llmax(mInFlightCounter-1,0); + + F32 seconds_to_wait; + mRetryPolicy->onFailure(getStatus(), getResponseHeaders()); + if (mRetryPolicy->shouldRetry(seconds_to_wait)) + { + llinfos << "retrying" << llendl; + doAfterInterval(boost::bind(&RequestAgentUpdateAppearanceResponder::sendRequest,this), + seconds_to_wait); + } + else + { + llwarns << "giving up after too many retries" << llendl; + } +} - LLPointer mRetryPolicy; -}; LLSD LLAppearanceMgr::dumpCOF() const { @@ -3253,53 +3369,9 @@ LLSD LLAppearanceMgr::dumpCOF() const return result; } -void LLAppearanceMgr::requestServerAppearanceUpdate(LLCurl::ResponderPtr responder_ptr) +void LLAppearanceMgr::requestServerAppearanceUpdate() { - if (gAgentAvatarp->isEditingAppearance()) - { - // don't send out appearance updates if in appearance editing mode - return; - } - - if (!gAgent.getRegion()) - { - llwarns << "Region not set, cannot request server appearance update" << llendl; - return; - } - if (gAgent.getRegion()->getCentralBakeVersion()==0) - { - llwarns << "Region does not support baking" << llendl; - } - std::string url = gAgent.getRegion()->getCapability("UpdateAvatarAppearance"); - if (url.empty()) - { - llwarns << "No cap for UpdateAvatarAppearance." << llendl; - return; - } - - LLSD body; - S32 cof_version = getCOFVersion(); - if (gSavedSettings.getBOOL("DebugAvatarExperimentalServerAppearanceUpdate")) - { - body = dumpCOF(); - } - else - { - body["cof_version"] = cof_version; - if (gSavedSettings.getBOOL("DebugForceAppearanceRequestFailure")) - { - body["cof_version"] = cof_version+999; - } - } - LL_DEBUGS("Avatar") << "request url " << url << " my_cof_version " << cof_version << llendl; - - if (!responder_ptr.get()) - { - responder_ptr = new RequestAgentUpdateAppearanceResponder; - } - LLHTTPClient::post(url, body, responder_ptr); - llassert(cof_version >= gAgentAvatarp->mLastUpdateRequestCOFVersion); - gAgentAvatarp->mLastUpdateRequestCOFVersion = cof_version; + mAppearanceResponder->onRequestRequested(); } class LLIncrementCofVersionResponder : public LLHTTPClient::Responder @@ -3635,7 +3707,8 @@ LLAppearanceMgr::LLAppearanceMgr(): mAttachmentInvLinkEnabled(false), mOutfitIsDirty(false), mOutfitLocked(false), - mIsInUpdateAppearanceFromCOF(false) + mIsInUpdateAppearanceFromCOF(false), + mAppearanceResponder(new RequestAgentUpdateAppearanceResponder) { LLOutfitObserver& outfit_observer = LLOutfitObserver::instance(); diff --git a/indra/newview/llappearancemgr.h b/indra/newview/llappearancemgr.h index b2917cced4..13a2a62717 100755 --- a/indra/newview/llappearancemgr.h +++ b/indra/newview/llappearancemgr.h @@ -38,6 +38,7 @@ class LLWearableHoldingPattern; class LLInventoryCallback; class LLOutfitUnLockTimer; +class RequestAgentUpdateAppearanceResponder; class LLAppearanceMgr: public LLSingleton { @@ -214,7 +215,7 @@ public: bool isInUpdateAppearanceFromCOF() { return mIsInUpdateAppearanceFromCOF; } - void requestServerAppearanceUpdate(LLCurl::ResponderPtr responder_ptr = NULL); + void requestServerAppearanceUpdate(); void incrementCofVersion(LLHTTPClient::ResponderPtr responder_ptr = NULL); @@ -255,6 +256,8 @@ private: bool mOutfitIsDirty; bool mIsInUpdateAppearanceFromCOF; // to detect recursive calls. + LLPointer mAppearanceResponder; + /** * Lock for blocking operations on outfit until server reply or timeout exceed * to avoid unsynchronized outfit state or performing duplicate operations. diff --git a/indra/newview/llhttpretrypolicy.cpp b/indra/newview/llhttpretrypolicy.cpp index 1512b46103..ae429f11f8 100755 --- a/indra/newview/llhttpretrypolicy.cpp +++ b/indra/newview/llhttpretrypolicy.cpp @@ -45,6 +45,11 @@ void LLAdaptiveRetryPolicy::init() mShouldRetry = true; } +void LLAdaptiveRetryPolicy::reset() +{ + init(); +} + bool LLAdaptiveRetryPolicy::getRetryAfter(const LLSD& headers, F32& retry_header_time) { return (headers.has(HTTP_IN_HEADER_RETRY_AFTER) diff --git a/indra/newview/llhttpretrypolicy.h b/indra/newview/llhttpretrypolicy.h index 5b1a1d79e0..cf79e0b401 100755 --- a/indra/newview/llhttpretrypolicy.h +++ b/indra/newview/llhttpretrypolicy.h @@ -53,6 +53,8 @@ public: virtual void onFailure(const LLCore::HttpResponse *response) = 0; virtual bool shouldRetry(F32& seconds_to_wait) const = 0; + + virtual void reset() = 0; }; // Very general policy with geometric back-off after failures, @@ -64,6 +66,8 @@ public: // virtual void onSuccess(); + + void reset(); // virtual void onFailure(S32 status, const LLSD& headers); diff --git a/indra/newview/llvoavatar.cpp b/indra/newview/llvoavatar.cpp index 2fd26eae80..74fb51b943 100755 --- a/indra/newview/llvoavatar.cpp +++ b/indra/newview/llvoavatar.cpp @@ -7014,7 +7014,15 @@ void LLVOAvatar::processAvatarAppearance( LLMessageSystem* mesgsys ) return; } - mLastUpdateReceivedCOFVersion = this_update_cof_version; + // No backsies zone - if we get here, the message should be valid and usable. + if (appearance_version > 0) + { + // Note: + // RequestAgentUpdateAppearanceResponder::onRequestRequested() + // assumes that cof version is only updated with server-bake + // appearance messages. + mLastUpdateReceivedCOFVersion = this_update_cof_version; + } setIsUsingServerBakes(appearance_version > 0); @@ -7061,6 +7069,7 @@ void LLVOAvatar::processAvatarAppearance( LLMessageSystem* mesgsys ) LL_DEBUGS("Avatar") << avString() << " handle visual params, num_params " << num_params << LL_ENDL; BOOL params_changed = FALSE; BOOL interp_params = FALSE; + S32 params_changed_count = 0; for( S32 i = 0; i < num_params; i++ ) { @@ -7070,12 +7079,15 @@ void LLVOAvatar::processAvatarAppearance( LLMessageSystem* mesgsys ) if (is_first_appearance_message || (param->getWeight() != newWeight)) { params_changed = TRUE; + params_changed_count++; if(is_first_appearance_message) { + //LL_DEBUGS("Avatar") << "param slam " << i << " " << newWeight << llendl; param->setWeight(newWeight, FALSE); } else { + //LL_DEBUGS("Avatar") << std::setprecision(5) << " param target " << i << " " << param->getWeight() << " -> " << newWeight << llendl; interp_params = TRUE; param->setAnimationTarget(newWeight, FALSE); } @@ -7087,6 +7099,7 @@ void LLVOAvatar::processAvatarAppearance( LLMessageSystem* mesgsys ) LL_DEBUGS("Avatar") << "Number of params in AvatarAppearance msg (" << num_params << ") does not match number of tweakable params in avatar xml file (" << expected_tweakable_count << "). Processing what we can. object: " << getID() << llendl; } + LL_DEBUGS("Avatar") << "Changed " << params_changed_count << " params" << llendl; if (params_changed) { if (interp_params) -- cgit v1.2.3