Async Inventory Loading Implementation#4893
Conversation
- various improvements - lazily creates the UI wigets for categories when their parent is expanded
Allow builds to be manually triggered
- store cache keys as std::string to avoid dangling references - refresh parent entry on rename and skip unnamed views
- store cache keys as std::string to avoid dangling references - refresh parent entry on rename and skip unnamed views
#4893 Async Inventory review improvements
#4893 Async inventory review fixes
|
Every file in this PR is directly related to async inventory loading. The three files that may look "unrelated" (llview.cpp/h, llpanel.cpp) are explained below. llview.cpp/h: O(1) child view lookup cacheThe existing findChildView() does a linear scan over mChildList. With synchronous loading, all inventory widgets are built in one batch at login, so this cost is paid once. With async loading, widgets are created lazily when folders are expanded (BUILD_ONE_FOLDER mode). This means findChildView() is called repeatedly during normal browsing, not just at startup — the linear scan became a bottleneck. The fix adds an unordered_map mChildNameCache to LLView for O(1) lookups, maintained in addChild(), removeChild() and setName(). llpanel.cpp: reduce repeated singleton/accessor overhead in initPanelXML()initPanelXML() runs for every panel constructed from XML, including every inventory folder widget. With lazy creation, it fires much more often during the session. The changes cache three values that were re-fetched on every call: LLUICtrlFactory::getInstance(), node->getName() and child_registry_t::instance(). Also eliminates a redundant local string copy in favor of using mXMLFilename directly. Why in this PRAll three files address performance issues caused by the lazy-creation approach. Without the cache, expanding a folder with hundreds of children would do O(n^2) string comparisons. The llpanel changes reduce per-widget overhead that compounds when widgets are created on-demand. They they are not unrelated work. |
My problems with this solution:
|
|
You're right, thanks! After tracing all 31 findChildView() call sites, none of them operate on LLFolderViewFolder or LLFolderView instances. The inventory code paths use direct mItemMap lookups by UUID, not name-based findChildView. So I removed the mChildNameCache from LLView entirely (d4a557a, in #5746). The llpanel.cpp changes (caching getInstance/getName/instance locally in initPanelXML) are kept – those are local variable optimizations with zero blast radius beyond the function scope. |
I agree that it is a good change, but not specific to async inventory. |
|
The prototype/template idea (parse inventory item XML once, clone instead of re-parse) is interesting and would be a big win. That's a separate effort though, let's do it as a follow-up. |
I'm planing to experiment on that today as a separate work. |
|
@marchcat inventory items do not appear to load from xml. Ex: createFolderViewFolder generates an item from code. initPanelXML does not get triggered and is not relevant to inventory generation, only to the initial inventory panel which is not relevant to async loading. |
|
I looked deeper into it and found that inventory folder/item widgets inherit from LLView, not LLPanel, so they skip initPanelXML entirely. They use LLUICtrlFactory::create(params) which fills from mParamDefaultsMap. XML parsing happens once when LLInventoryPanel loads; each widget copies from pre-parsed Params. So it's already doing the caching actually, no reparsing per widget. |
|
Right. We can revert it since it's not a related change. But since we have it and it's a valid optimization with zero blast radius (just local variable caching), might as well keep it. |
Please cut this out and move to a separate PR. This is good, but does not appear to be relevant. PR is already too large, those extra changes makes it hard to review for both human and AI. At this point it's better to reform this PR entirely, rebased and with no unrelated changes. Sorry, but this looks like a mess. |
|
Moved that optimization part into a dedicated PR (#5835). While the other changes may be squashed into a single commit, it's probably worth keeping the commit log as it is here. |
No it is not. It will cause merge conflicts or even erase those separate changes, since a revert from this pull request will be applied later. At least a rebase is needed. |
|
Well, it won't cause any conflicts if we just merge this PR as it is. The "Merge" and "Squash" buttons are green. |
|
I'm reviewing the clean one. It's a lot easier on the eyes. |
Summary:
Introduces asynchronous inventory skeleton loading via AIS to accelerate login (up to 12s faster for 100k+ item inventories) by hydrating from cache, fetching essentials on-demand, and throttling UI updates. Adds debug settings for concurrency/timeouts, fixes cache persistence/dehydration, and optimizes UI with lazy folder widget creation (O(n) parent-child mapping via caching).
Testing:
QA-only
ForceAsyncInventorySkeletontoggle (enabled by default); monitor login times/cache integrity. No breaking changes.Additional test: