From 5759be7cdf0f8fe0745265b85bab8c37bd2335be Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Wed, 17 Aug 2022 18:06:51 +0300 Subject: SL-17834 MP window freezes when loading items from Inventory 1. Do not process all marketpalce changes at once, schedule them for later 2. Do not notifyObservers from notifyObservers --- indra/newview/llmarketplacefunctions.cpp | 126 ++++++++++++++++++++----------- 1 file changed, 84 insertions(+), 42 deletions(-) (limited to 'indra/newview/llmarketplacefunctions.cpp') diff --git a/indra/newview/llmarketplacefunctions.cpp b/indra/newview/llmarketplacefunctions.cpp index 044c76ce2c..74616df079 100644 --- a/indra/newview/llmarketplacefunctions.cpp +++ b/indra/newview/llmarketplacefunctions.cpp @@ -30,6 +30,7 @@ #include "llagent.h" #include "llbufferstream.h" +#include "llcallbacklist.h" #include "llinventoryfunctions.h" #include "llinventoryobserver.h" #include "llnotificationsutil.h" @@ -605,20 +606,62 @@ public: LLMarketplaceInventoryObserver() {} virtual ~LLMarketplaceInventoryObserver() {} virtual void changed(U32 mask); + +private: + static void onIdleProcessQueue(void *userdata); + + // doesn't hold just marketplace related ids + static std::set sAddQueue; + static std::set sStructureQueue; }; +std::set LLMarketplaceInventoryObserver::sAddQueue; +std::set LLMarketplaceInventoryObserver::sStructureQueue; + void LLMarketplaceInventoryObserver::changed(U32 mask) { - // When things are added to the marketplace, we might need to re-validate and fix the containing listings - if (mask & LLInventoryObserver::ADD) + bool idle_running = !sAddQueue.empty() && !sStructureQueue.empty(); + bool needs_idle = false; + if (mask & LLInventoryObserver::ADD && LLMarketplaceData::instance().hasValidationWaiting()) { + // When things are added to the marketplace, we might need to re-validate and fix the containing listings + // just add whole list even if it contains items and non-marketplace folders const std::set& changed_items = gInventory.getChangedIDs(); - - std::set::const_iterator id_it = changed_items.begin(); - std::set::const_iterator id_end = changed_items.end(); + sAddQueue.insert(changed_items.begin(), changed_items.end()); + needs_idle = true; + } + + if (mask & (LLInventoryObserver::INTERNAL | LLInventoryObserver::STRUCTURE)) + { + // When things are changed in the inventory, this can trigger a host of changes in the marketplace listings folder: + // * stock counts changing : no copy items coming in and out will change the stock count on folders + // * version and listing folders : moving those might invalidate the marketplace data itself + // Since we should cannot raise inventory change while the observer is called (the list will be cleared + // once observers are called) we need to raise a flag in the inventory to signal that things have been dirtied. + const std::set& changed_items = gInventory.getChangedIDs(); + sStructureQueue.insert(changed_items.begin(), changed_items.end()); + needs_idle = true; + } + + if (!idle_running && needs_idle) + { + gIdleCallbacks.addFunction(onIdleProcessQueue, NULL); + } +} + +void LLMarketplaceInventoryObserver::onIdleProcessQueue(void *userdata) +{ + U64 start_time = LLTimer::getTotalTime(); // microseconds + const U64 MAX_PROCESSING_TIME = 1000; + U64 stop_time = start_time + MAX_PROCESSING_TIME; + + if (!sAddQueue.empty()) + { + std::set::const_iterator id_it = sAddQueue.begin(); + std::set::const_iterator id_end = sAddQueue.end(); // First, count the number of items in this list... S32 count = 0; - for (;id_it != id_end; ++id_it) + for (; id_it != id_end; ++id_it) { LLInventoryObject* obj = gInventory.getObject(*id_it); if (obj && (LLAssetType::AT_CATEGORY != obj->getType())) @@ -629,56 +672,55 @@ void LLMarketplaceInventoryObserver::changed(U32 mask) // Then, decrement the folders of that amount // Note that of all of those, only one folder will be a listing folder (if at all). // The other will be ignored by the decrement method. - id_it = changed_items.begin(); - for (;id_it != id_end; ++id_it) + id_it = sAddQueue.begin(); + for (; id_it != id_end; ++id_it) { LLInventoryObject* obj = gInventory.getObject(*id_it); if (obj && (LLAssetType::AT_CATEGORY == obj->getType())) { - LLMarketplaceData::instance().decrementValidationWaiting(obj->getUUID(),count); + // can trigger notifyObservers + LLMarketplaceData::instance().decrementValidationWaiting(obj->getUUID(), count); } } - } - - // When things are changed in the inventory, this can trigger a host of changes in the marketplace listings folder: - // * stock counts changing : no copy items coming in and out will change the stock count on folders - // * version and listing folders : moving those might invalidate the marketplace data itself - // Since we should cannot raise inventory change while the observer is called (the list will be cleared - // once observers are called) we need to raise a flag in the inventory to signal that things have been dirtied. - - if (mask & (LLInventoryObserver::INTERNAL | LLInventoryObserver::STRUCTURE)) - { - const std::set& changed_items = gInventory.getChangedIDs(); - - std::set::const_iterator id_it = changed_items.begin(); - std::set::const_iterator id_end = changed_items.end(); - for (;id_it != id_end; ++id_it) + sAddQueue.clear(); + } + + std::set::const_iterator id_it = sStructureQueue.begin(); + while (id_it != sStructureQueue.end() && LLTimer::getTotalTime() < stop_time) + { + LLInventoryObject* obj = gInventory.getObject(*id_it); + if (obj) { - LLInventoryObject* obj = gInventory.getObject(*id_it); - if (obj) + if (LLAssetType::AT_CATEGORY == obj->getType()) { - if (LLAssetType::AT_CATEGORY == obj->getType()) + // If it's a folder known to the marketplace, let's check it's in proper shape + if (LLMarketplaceData::instance().isListed(*id_it) || LLMarketplaceData::instance().isVersionFolder(*id_it)) { - // If it's a folder known to the marketplace, let's check it's in proper shape - if (LLMarketplaceData::instance().isListed(*id_it) || LLMarketplaceData::instance().isVersionFolder(*id_it)) - { - LLInventoryCategory* cat = (LLInventoryCategory*)(obj); - validate_marketplacelistings(cat); - } + LLInventoryCategory* cat = (LLInventoryCategory*)(obj); + // can trigger notifyObservers + // can cause more structural changes + validate_marketplacelistings(cat); } - else + } + else + { + // If it's not a category, it's an item... + LLInventoryItem* item = (LLInventoryItem*)(obj); + // If it's a no copy item, we may need to update the label count of marketplace listings + if (!item->getPermissions().allowOperationBy(PERM_COPY, gAgent.getID(), gAgent.getGroupID())) { - // If it's not a category, it's an item... - LLInventoryItem* item = (LLInventoryItem*)(obj); - // If it's a no copy item, we may need to update the label count of marketplace listings - if (!item->getPermissions().allowOperationBy(PERM_COPY, gAgent.getID(), gAgent.getGroupID())) - { - LLMarketplaceData::instance().setDirtyCount(); - } + LLMarketplaceData::instance().setDirtyCount(); } } } - } + id_it = sStructureQueue.erase(id_it); + } + + if (LLApp::isExiting() || sStructureQueue.empty()) + { + // Nothing to do anymore + gIdleCallbacks.deleteFunction(onIdleProcessQueue, NULL); + } } // Tuple == Item -- cgit v1.2.3 From 6717f4a609e4c158b78069353c0a43fe2fe79af7 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Thu, 18 Aug 2022 12:06:19 +0300 Subject: SL-17834 MP window freezes when loading items from Inventory #2 --- indra/newview/llmarketplacefunctions.cpp | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'indra/newview/llmarketplacefunctions.cpp') diff --git a/indra/newview/llmarketplacefunctions.cpp b/indra/newview/llmarketplacefunctions.cpp index 74616df079..a5f1622ab2 100644 --- a/indra/newview/llmarketplacefunctions.cpp +++ b/indra/newview/llmarketplacefunctions.cpp @@ -613,22 +613,21 @@ private: // doesn't hold just marketplace related ids static std::set sAddQueue; static std::set sStructureQueue; + static bool sProcessingQueue; }; std::set LLMarketplaceInventoryObserver::sAddQueue; std::set LLMarketplaceInventoryObserver::sStructureQueue; +bool LLMarketplaceInventoryObserver::sProcessingQueue = false; void LLMarketplaceInventoryObserver::changed(U32 mask) { - bool idle_running = !sAddQueue.empty() && !sStructureQueue.empty(); - bool needs_idle = false; if (mask & LLInventoryObserver::ADD && LLMarketplaceData::instance().hasValidationWaiting()) { // When things are added to the marketplace, we might need to re-validate and fix the containing listings // just add whole list even if it contains items and non-marketplace folders const std::set& changed_items = gInventory.getChangedIDs(); sAddQueue.insert(changed_items.begin(), changed_items.end()); - needs_idle = true; } if (mask & (LLInventoryObserver::INTERNAL | LLInventoryObserver::STRUCTURE)) @@ -640,12 +639,13 @@ void LLMarketplaceInventoryObserver::changed(U32 mask) // once observers are called) we need to raise a flag in the inventory to signal that things have been dirtied. const std::set& changed_items = gInventory.getChangedIDs(); sStructureQueue.insert(changed_items.begin(), changed_items.end()); - needs_idle = true; } - if (!idle_running && needs_idle) + if (!sProcessingQueue && (!sAddQueue.empty() || !sStructureQueue.empty())) { gIdleCallbacks.addFunction(onIdleProcessQueue, NULL); + // can do without sProcessingQueue, but it's usufull for simplicity and reliability + sProcessingQueue = true; } } @@ -657,8 +657,13 @@ void LLMarketplaceInventoryObserver::onIdleProcessQueue(void *userdata) if (!sAddQueue.empty()) { - std::set::const_iterator id_it = sAddQueue.begin(); - std::set::const_iterator id_end = sAddQueue.end(); + // Make a copy of sAddQueue since decrementValidationWaiting + // can theoretically add more items + std::set add_queue(sAddQueue); + sAddQueue.clear(); + + std::set::const_iterator id_it = add_queue.begin(); + std::set::const_iterator id_end = add_queue.end(); // First, count the number of items in this list... S32 count = 0; for (; id_it != id_end; ++id_it) @@ -672,7 +677,7 @@ void LLMarketplaceInventoryObserver::onIdleProcessQueue(void *userdata) // Then, decrement the folders of that amount // Note that of all of those, only one folder will be a listing folder (if at all). // The other will be ignored by the decrement method. - id_it = sAddQueue.begin(); + id_it = add_queue.begin(); for (; id_it != id_end; ++id_it) { LLInventoryObject* obj = gInventory.getObject(*id_it); @@ -682,12 +687,11 @@ void LLMarketplaceInventoryObserver::onIdleProcessQueue(void *userdata) LLMarketplaceData::instance().decrementValidationWaiting(obj->getUUID(), count); } } - sAddQueue.clear(); } - std::set::const_iterator id_it = sStructureQueue.begin(); - while (id_it != sStructureQueue.end() && LLTimer::getTotalTime() < stop_time) + while (!sStructureQueue.empty() && LLTimer::getTotalTime() < stop_time) { + std::set::const_iterator id_it = sStructureQueue.begin(); LLInventoryObject* obj = gInventory.getObject(*id_it); if (obj) { @@ -713,13 +717,17 @@ void LLMarketplaceInventoryObserver::onIdleProcessQueue(void *userdata) } } } - id_it = sStructureQueue.erase(id_it); + + // sStructureQueue could have been modified in validate_marketplacelistings + // adding items does not invalidate existing iterator + sStructureQueue.erase(id_it); } - if (LLApp::isExiting() || sStructureQueue.empty()) + if (LLApp::isExiting() || (sAddQueue.empty() && sStructureQueue.empty())) { // Nothing to do anymore gIdleCallbacks.deleteFunction(onIdleProcessQueue, NULL); + sProcessingQueue = false; } } -- cgit v1.2.3