SL-14399: Ditch overflow queue LLViewerAssetStorage::mCoroWaitList.#28
Merged
nat-goodspeed merged 1 commit intocontributefrom Dec 8, 2022
Merged
SL-14399: Ditch overflow queue LLViewerAssetStorage::mCoroWaitList.#28nat-goodspeed merged 1 commit intocontributefrom
nat-goodspeed merged 1 commit intocontributefrom
Conversation
mCoroWaitList was introduced to prevent an assertion failure crash: LLCoprocedureManager never expects to fill LLCoprocedurePool::mPendingCoprocs queue. The queue limit was arbitrarily set to 4096 some years ago, but in practice LLViewerAssetStorage can post way more requests than that. LLViewerAssetStorage checked whether the target LLCoprocedureManager pool's queue looked close to full, and if so posted the pending request to its mCoroWaitList instead. But then it had to override the base LLAssetStorage method checkForTimeouts() to continually check whether pending tasks could be moved from mCoroWaitList to LLCoprocedureManager. A simpler solution is to enlarge LLCorpocedureManager::DEFAULT_QUEUE_SIZE, the upper limit on mPendingCoprocs. Since mCoroWaitList was an unlimited queue, making DEFAULT_QUEUE_SIZE "very large" does not increase the risk of runaway memory consumption.
akleshchev
approved these changes
Dec 7, 2022
akleshchev
reviewed
Dec 7, 2022
| // SL-14399: When we teleport to a brand-new simulator, the coprocedure queue | ||
| // gets absolutely slammed with fetch requests. Make this queue effectively | ||
| // unlimited. | ||
| const U32 LLCoprocedureManager::DEFAULT_QUEUE_SIZE = 1024*1024; |
Contributor
There was a problem hiding this comment.
The biggest offenders probably aren't teleports but landmarks and gestures. Viewer needs all landmark assets loaded to let search and landmarking work and there can be thousands of landmarks in some inventories.
Rider-Linden
approved these changes
Dec 7, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mCoroWaitList was introduced to prevent an assertion failure crash: LLCoprocedureManager never expects to fill its LLCoprocedurePool::mPendingCoprocs queue. The queue limit was arbitrarily set to 4096 some years ago, but in practice LLViewerAssetStorage can post way more requests than that.
LLViewerAssetStorage checked whether the target LLCoprocedureManager pool's queue looked close to full, and if so posted the pending request to its mCoroWaitList instead. But then it had to override the base LLAssetStorage method checkForTimeouts() to continually check whether pending tasks could be moved from mCoroWaitList to LLCoprocedureManager.
A simpler solution is to enlarge LLCorpocedureManager::DEFAULT_QUEUE_SIZE, the upper limit on mPendingCoprocs. Since mCoroWaitList was an unlimited queue, making DEFAULT_QUEUE_SIZE "very large" does not increase the risk of runaway memory consumption.