Unload render assets from RAM #10520

JMS55 · 2023-11-12T05:12:58Z

Objective

No point in keeping Meshes/Images in RAM once they're going to be sent to the GPU, and kept in VRAM. This saves a significant amount of memory (several GBs) on scenes like bistro.
References
- Mesh: added static and dynamic mesh for less runtime memory usage #1782
- Bevy Asset V2 #8624

Solution

Augment RenderAsset with the capability to unload the underlying asset after extracting to the render world.
Mesh/Image now have a cpu_persistent_access field. If this field is RenderAssetPersistencePolicy::Unload, the asset will be unloaded from Assets.
A new AssetEvent is sent upon dropping the last strong handle for the asset, which signals to the RenderAsset to remove the GPU version of the asset.

Changelog

Added AssetEvent::NoLongerUsed and AssetEvent::is_no_longer_used(). This event is sent when the last strong handle of an asset is dropped.
Rewrote the API for RenderAsset to allow for unloading the asset data from the CPU.
Added RenderAssetPersistencePolicy.
Added Mesh::cpu_persistent_access for memory savings when the asset is not needed except for on the GPU.
Added Image::cpu_persistent_access for memory savings when the asset is not needed except for on the GPU.
Added ImageLoaderSettings::cpu_persistent_access.
Added ExrTextureLoaderSettings.
Added HdrTextureLoaderSettings.

Migration Guide

Asset loaders (GLTF, etc) now load meshes and textures without cpu_persistent_access. These assets will be removed from Assets<Mesh> and Assets<Image> once RenderAssets<Mesh> and RenderAssets<Image> contain the GPU versions of these assets, in order to reduce memory usage. If you require access to the asset data from the CPU in future frames after the GLTF asset has been loaded, modify all dependent Mesh and Image assets and set cpu_persistent_access to RenderAssetPersistencePolicy::Keep.
Mesh now requires a new cpu_persistent_access field. Set it to RenderAssetPersistencePolicy::Keep to mimic the previous behavior.
Image now requires a new cpu_persistent_access field. Set it to RenderAssetPersistencePolicy::Keep to mimic the previous behavior.
MorphTargetImage::new() now requires a new cpu_persistent_access parameter. Set it to RenderAssetPersistencePolicy::Keep to mimic the previous behavior.
DynamicTextureAtlasBuilder::add_texture() now requires that the TextureAtlas you pass has an Image with cpu_persistent_access: RenderAssetPersistencePolicy::Keep. Ensure you construct the image properly for the texture atlas.
The RenderAsset trait has significantly changed, and requires adapting your existing implementations.
- The trait now requires Clone.
- The ExtractedAsset associated type has been removed (the type itself is now extracted).
- The signature of prepare_asset() is slightly different
- A new persistence_policy() method is now required (return RenderAssetPersistencePolicy::Unload to match the previous behavior).
Match on the new NoLongerUsed variant for exhaustive matches of AssetEvent.

tim-blackbird

Bikeshed over defaults for mutable/immutable (I think we can agree assets loaded from disk should default to immutable, at least)

IMO unloading from the cpu is the right default in both cases.

We still have to clone the entire Mesh/Image data to the render world once. Ideally we only clone an Arc or something cheap.

Easy enough to solve by making RenderAsset::extract_asset take self instead of &self, but would require a Clone bound for RenderAsset so we can clone inside the extract_render_asset system when we don't want to unload the asset.
That might cause problems for RenderAsset uses outside of Bevy.

No way to re-load the asset to put it's data back in RAM once unloaded.

It's fine to punt this to a future PR

crates/bevy_render/src/mesh/mesh/mod.rs

tim-blackbird · 2023-11-12T17:40:25Z

crates/bevy_render/src/mesh/mesh/mod.rs

@@ -180,13 +181,14 @@ impl Mesh {
    /// Construct a new mesh. You need to provide a [`PrimitiveTopology`] so that the
    /// renderer knows how to treat the vertex data. Most of the time this will be
    /// [`PrimitiveTopology::TriangleList`].
-    pub fn new(primitive_topology: PrimitiveTopology) -> Self {
+    pub fn new(primitive_topology: PrimitiveTopology, mutable: bool) -> Self {


I think this would be nicer as a separate method instead, like with_cpu_access(), similar to the existing methods.

Not sure how I feel. I wanted users to be aware of this. I'm worried if we make it a separate method they'll not realize, and then be confused why their asset is "missing" in their systems later on.

JMS55 · 2023-11-12T20:13:29Z

Easy enough to solve by making RenderAsset::extract_asset take self instead of &self, but would require a Clone bound for RenderAsset so we can clone inside the extract_render_asset system when we don't want to unload the asset.

I don't see how this would fix the expensive clone of all mesh/image bytes to the render world during extract.

tim-blackbird · 2023-11-12T22:16:13Z

If extract_asset() took self instead of &self then the clone inside the extract_render_asset system only needs to happen if unload_after_extract() returns false.

if asset.unload_after_extract() {
    // Move the asset out of the collection
    if let Some(asset) = assets.remove_untracked(id) {
        extracted_assets.push((id, asset.extract_asset()));
    }
} else {
    // Clone the asset reference
    extracted_assets.push((id, asset.clone().extract_asset()));
}

JMS55 · 2023-11-12T22:33:29Z

Oooh I see, good idea! Let me implement that :)

JMS55 · 2023-11-12T22:49:10Z

Removed the ExtractedAsset type. Nothing actually used it, and it lets us avoid an extra clone for persistent assets.

Friz64 · 2023-11-14T05:33:23Z

It might be nice to make this an enum instead of an boolean, so that code is easier to understand when looking at it at a glance.

…ender_asset

…t does not currently exist

crates/bevy_asset/src/handle.rs

crates/bevy_asset/src/assets.rs

…#11205) # Objective - after #10520, UI can't display images or text with a background color ## Solution - don't filter out UI nodes with an unloaded image

…1383) # Objective After #10520, I was experiencing seriously degraded performance that ended up being due to never-drained `AssetEvent` events causing havoc inside `extract_render_asset::<A>`. The same events being read over and over again meant the same assets were being prepared every frame for eternity. For what it's worth, I was noticing this on a static scene about every 3rd or so time running my project. * References #10520 * Fixes #11240 Why these events aren't sometimes drained between frames is beyond me and perhaps worthy of another investigation, but the approach in this PR effectively restores the original cached `EventReader` behavior (which fixes it). ## Solution I followed the [`CachedSystemState` example](https://github.com/bevyengine/bevy/blob/3a666cab23eab5e83552ae074f47d8663221311e/crates/bevy_ecs/src/system/function_system.rs#L155) to make sure that the `EventReader` state is cached between frames like it used to be when it was an argument of `extract_render_asset::<A>`.

# Objective Right now, all assets in the main world get extracted and prepared in the render world (if the asset's using the RenderAssetPlugin). This is unfortunate for two cases: 1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual `Image` assets that make up the atlas are cloned and prepared individually when there's no reason for them to be. The atlas textures are built on the CPU in the main world. *There can be hundreds of images that get prepared for rendering only not to be used.* 2. If one loads an Image and needs to transform it in a system before rendering it, kind of like the [decompression example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120), there's a price paid for extracting & preparing the asset that's not intended to be rendered yet. ------ * References #10520 * References #1782 ## Solution This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt that the objective with the parameter is so similar in nature to wgpu's [`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html) and [`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html), that it may as well be just like that. ```rust // This asset only needs to be in the main world. Don't extract and prepare it. RenderAssetUsages::MAIN_WORLD // Keep this asset in the main world and RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD // This asset is only needed in the render world. Remove it from the asset server once extracted. RenderAssetUsages::RENDER_WORLD ``` ### Alternate Solution I considered introducing a third field to `RenderAssetPersistencePolicy` enum: ```rust enum RenderAssetPersistencePolicy { /// Keep the asset in the main world after extracting to the render world. Keep, /// Remove the asset from the main world after extracting to the render world. Unload, /// This doesn't need to be in the render world at all. NoExtract, // <----- } ``` Functional, but this seemed like shoehorning. Another option is renaming the enum to something like: ```rust enum RenderAssetExtractionPolicy { /// Extract the asset and keep it in the main world. Extract, /// Remove the asset from the main world after extracting to the render world. ExtractAndUnload, /// This doesn't need to be in the render world at all. NoExtract, } ``` I think this last one could be a good option if the bitflags are too clunky. ## Migration Guide * `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD | RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`) * `RenderAssetPersistencePolicy::Unload` → `RenderAssetUsage::RENDER_WORLD` * For types implementing the `RenderAsset` trait, change `fn persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn asset_usage(&self) -> RenderAssetUsages`. * Change any references to `cpu_persistent_access` (`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`). This applies to `Image`, `Mesh`, and a few other types.

…bevyengine#11205) # Objective - after bevyengine#10520, UI can't display images or text with a background color ## Solution - don't filter out UI nodes with an unloaded image

…vyengine#11383) # Objective After bevyengine#10520, I was experiencing seriously degraded performance that ended up being due to never-drained `AssetEvent` events causing havoc inside `extract_render_asset::<A>`. The same events being read over and over again meant the same assets were being prepared every frame for eternity. For what it's worth, I was noticing this on a static scene about every 3rd or so time running my project. * References bevyengine#10520 * Fixes bevyengine#11240 Why these events aren't sometimes drained between frames is beyond me and perhaps worthy of another investigation, but the approach in this PR effectively restores the original cached `EventReader` behavior (which fixes it). ## Solution I followed the [`CachedSystemState` example](https://github.com/bevyengine/bevy/blob/3a666cab23eab5e83552ae074f47d8663221311e/crates/bevy_ecs/src/system/function_system.rs#L155) to make sure that the `EventReader` state is cached between frames like it used to be when it was an argument of `extract_render_asset::<A>`.

# Objective Right now, all assets in the main world get extracted and prepared in the render world (if the asset's using the RenderAssetPlugin). This is unfortunate for two cases: 1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual `Image` assets that make up the atlas are cloned and prepared individually when there's no reason for them to be. The atlas textures are built on the CPU in the main world. *There can be hundreds of images that get prepared for rendering only not to be used.* 2. If one loads an Image and needs to transform it in a system before rendering it, kind of like the [decompression example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120), there's a price paid for extracting & preparing the asset that's not intended to be rendered yet. ------ * References bevyengine#10520 * References bevyengine#1782 ## Solution This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt that the objective with the parameter is so similar in nature to wgpu's [`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html) and [`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html), that it may as well be just like that. ```rust // This asset only needs to be in the main world. Don't extract and prepare it. RenderAssetUsages::MAIN_WORLD // Keep this asset in the main world and RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD // This asset is only needed in the render world. Remove it from the asset server once extracted. RenderAssetUsages::RENDER_WORLD ``` ### Alternate Solution I considered introducing a third field to `RenderAssetPersistencePolicy` enum: ```rust enum RenderAssetPersistencePolicy { /// Keep the asset in the main world after extracting to the render world. Keep, /// Remove the asset from the main world after extracting to the render world. Unload, /// This doesn't need to be in the render world at all. NoExtract, // <----- } ``` Functional, but this seemed like shoehorning. Another option is renaming the enum to something like: ```rust enum RenderAssetExtractionPolicy { /// Extract the asset and keep it in the main world. Extract, /// Remove the asset from the main world after extracting to the render world. ExtractAndUnload, /// This doesn't need to be in the render world at all. NoExtract, } ``` I think this last one could be a good option if the bitflags are too clunky. ## Migration Guide * `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD | RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`) * `RenderAssetPersistencePolicy::Unload` → `RenderAssetUsage::RENDER_WORLD` * For types implementing the `RenderAsset` trait, change `fn persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn asset_usage(&self) -> RenderAssetUsages`. * Change any references to `cpu_persistent_access` (`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`). This applies to `Image`, `Mesh`, and a few other types.

- No point in keeping Meshes/Images in RAM once they're going to be sent to the GPU, and kept in VRAM. This saves a _significant_ amount of memory (several GBs) on scenes like bistro. - References - bevyengine#1782 - bevyengine#8624 - Augment RenderAsset with the capability to unload the underlying asset after extracting to the render world. - Mesh/Image now have a cpu_persistent_access field. If this field is RenderAssetPersistencePolicy::Unload, the asset will be unloaded from Assets<T>. - A new AssetEvent is sent upon dropping the last strong handle for the asset, which signals to the RenderAsset to remove the GPU version of the asset. --- - Added `AssetEvent::NoLongerUsed` and `AssetEvent::is_no_longer_used()`. This event is sent when the last strong handle of an asset is dropped. - Rewrote the API for `RenderAsset` to allow for unloading the asset data from the CPU. - Added `RenderAssetPersistencePolicy`. - Added `Mesh::cpu_persistent_access` for memory savings when the asset is not needed except for on the GPU. - Added `Image::cpu_persistent_access` for memory savings when the asset is not needed except for on the GPU. - Added `ImageLoaderSettings::cpu_persistent_access`. - Added `ExrTextureLoaderSettings`. - Added `HdrTextureLoaderSettings`. - Asset loaders (GLTF, etc) now load meshes and textures without `cpu_persistent_access`. These assets will be removed from `Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and `RenderAssets<Image>` contain the GPU versions of these assets, in order to reduce memory usage. If you require access to the asset data from the CPU in future frames after the GLTF asset has been loaded, modify all dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to `RenderAssetPersistencePolicy::Keep`. - `Mesh` now requires a new `cpu_persistent_access` field. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the previous behavior. - `Image` now requires a new `cpu_persistent_access` field. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the previous behavior. - `MorphTargetImage::new()` now requires a new `cpu_persistent_access` parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the previous behavior. - `DynamicTextureAtlasBuilder::add_texture()` now requires that the `TextureAtlas` you pass has an `Image` with `cpu_persistent_access: RenderAssetPersistencePolicy::Keep`. Ensure you construct the image properly for the texture atlas. - The `RenderAsset` trait has significantly changed, and requires adapting your existing implementations. - The trait now requires `Clone`. - The `ExtractedAsset` associated type has been removed (the type itself is now extracted). - The signature of `prepare_asset()` is slightly different - A new `persistence_policy()` method is now required (return RenderAssetPersistencePolicy::Unload to match the previous behavior). - Match on the new `NoLongerUsed` variant for exhaustive matches of `AssetEvent`.

# Objective - Since bevyengine#10520, assets are unloaded from RAM by default. This breaks a number of scenario: - using `load_folder` - loading a gltf, then going through its mesh to transform them / compute a collider / ... - any assets/subassets scenario should be `Keep` as you can't know what the user will do with the assets - android suspension, where GPU memory is unloaded - Alternative to bevyengine#11202 ## Solution - Keep assets on CPU memory by default

…vyengine#11383) # Objective After bevyengine#10520, I was experiencing seriously degraded performance that ended up being due to never-drained `AssetEvent` events causing havoc inside `extract_render_asset::<A>`. The same events being read over and over again meant the same assets were being prepared every frame for eternity. For what it's worth, I was noticing this on a static scene about every 3rd or so time running my project. * References bevyengine#10520 * Fixes bevyengine#11240 Why these events aren't sometimes drained between frames is beyond me and perhaps worthy of another investigation, but the approach in this PR effectively restores the original cached `EventReader` behavior (which fixes it). ## Solution I followed the [`CachedSystemState` example](https://github.com/bevyengine/bevy/blob/3a666cab23eab5e83552ae074f47d8663221311e/crates/bevy_ecs/src/system/function_system.rs#L155) to make sure that the `EventReader` state is cached between frames like it used to be when it was an argument of `extract_render_asset::<A>`.

Right now, all assets in the main world get extracted and prepared in the render world (if the asset's using the RenderAssetPlugin). This is unfortunate for two cases: 1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual `Image` assets that make up the atlas are cloned and prepared individually when there's no reason for them to be. The atlas textures are built on the CPU in the main world. *There can be hundreds of images that get prepared for rendering only not to be used.* 2. If one loads an Image and needs to transform it in a system before rendering it, kind of like the [decompression example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120), there's a price paid for extracting & preparing the asset that's not intended to be rendered yet. ------ * References bevyengine#10520 * References bevyengine#1782 This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt that the objective with the parameter is so similar in nature to wgpu's [`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html) and [`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html), that it may as well be just like that. ```rust // This asset only needs to be in the main world. Don't extract and prepare it. RenderAssetUsages::MAIN_WORLD // Keep this asset in the main world and RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD // This asset is only needed in the render world. Remove it from the asset server once extracted. RenderAssetUsages::RENDER_WORLD ``` I considered introducing a third field to `RenderAssetPersistencePolicy` enum: ```rust enum RenderAssetPersistencePolicy { /// Keep the asset in the main world after extracting to the render world. Keep, /// Remove the asset from the main world after extracting to the render world. Unload, /// This doesn't need to be in the render world at all. NoExtract, // <----- } ``` Functional, but this seemed like shoehorning. Another option is renaming the enum to something like: ```rust enum RenderAssetExtractionPolicy { /// Extract the asset and keep it in the main world. Extract, /// Remove the asset from the main world after extracting to the render world. ExtractAndUnload, /// This doesn't need to be in the render world at all. NoExtract, } ``` I think this last one could be a good option if the bitflags are too clunky. * `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD | RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`) * `RenderAssetPersistencePolicy::Unload` → `RenderAssetUsage::RENDER_WORLD` * For types implementing the `RenderAsset` trait, change `fn persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn asset_usage(&self) -> RenderAssetUsages`. * Change any references to `cpu_persistent_access` (`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`). This applies to `Image`, `Mesh`, and a few other types.