Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid exclusive lock during update_asset_storage #909

Merged
merged 2 commits into from
Nov 22, 2020
Merged

avoid exclusive lock during update_asset_storage #909

merged 2 commits into from
Nov 22, 2020

Conversation

blamelessgames
Copy link
Contributor

noticed a coarsely held exclusive lock in update_asset_storage that was causing contention - occasionally one version or another of the wrapping system would be held for 10+ms in my profiling.

narrowed the lock scope to the point of use. if this solution seems too quick-and-dirty i'm willing to re-examine

@cart
Copy link
Member

cart commented Nov 22, 2020

yeah asset lifecycle events should be pretty rare, so grabbing the lock on each event doesn't seem terrible. The tradeoff is that we get way more "lock-ey" when we load many assets at once. We could optimize that by adding a let asset_sources: Option<RwLockWriteGuard<HashMap<SourcePathId, SourceInfo>> = None right before the loop starts, then storing the guard in there, which should let us grab the guard once across all items processed.

@Moxinilian Moxinilian added A-Assets Load files from disk to use for things like images, models, and sounds C-Performance A change motivated by improving speed, memory usage or compile times labels Nov 22, 2020
@cart cart merged commit 01ba7c4 into bevyengine:master Nov 22, 2020
@ambeeeeee ambeeeeee mentioned this pull request Nov 28, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants