From b70d8253c433855715d1be8201eb8d1fdaee10a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Mon, 26 Apr 2021 00:26:11 +0200 Subject: [PATCH 1/5] mark `load` and `load_folder` as must_use --- crates/bevy_asset/src/asset_server.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 0f4ae4081ab67..72a18b9c291a2 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -224,6 +224,7 @@ impl AssetServer { /// The name of the asset folder is set inside the /// [`AssetServerSettings`](crate::AssetServerSettings) resource. The default name is /// `"assets"`. + #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] pub fn load<'a, T: Asset, P: Into>>(&self, path: P) -> Handle { self.load_untyped(path).typed() } @@ -324,7 +325,8 @@ impl AssetServer { let type_uuid = loaded_asset.value.as_ref().unwrap().type_uuid(); source_info.asset_types.insert(label_id, type_uuid); for dependency in loaded_asset.dependencies.iter() { - self.load_untyped(dependency.clone()); + // another handle already exists created from the asset path + let _ = self.load_untyped(dependency.clone()); } } @@ -336,6 +338,7 @@ impl AssetServer { Ok(asset_path_id) } + #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] pub fn load_untyped<'a, P: Into>>(&self, path: P) -> HandleUntyped { let handle_id = self.load_untracked(path.into(), false); self.get_handle_untyped(handle_id) @@ -355,6 +358,7 @@ impl AssetServer { asset_path.into() } + #[must_use = "not using the returned strong handles may result in the unexpected release of the assets"] pub fn load_folder>( &self, path: P, From 5a83a0c04d9953e0843faca0fc42df44f2e6d54c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Mon, 26 Apr 2021 01:32:13 +0200 Subject: [PATCH 2/5] return fast if asset is still loading --- crates/bevy_asset/src/asset_server.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 72a18b9c291a2..4c51de3dab288 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -254,11 +254,12 @@ impl AssetServer { }), }; - // if asset is already loaded (or is loading), don't load again + // if asset is already loaded or is loading, don't load again if !force - && source_info + && (source_info .committed_assets .contains(&asset_path_id.label_id()) + || source_info.load_state == LoadState::Loading) { return Ok(asset_path_id); } From 6c260e089439b7a83b065570f9f7569ddbc1398f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Mon, 26 Apr 2021 01:32:36 +0200 Subject: [PATCH 3/5] remove unneeded if level --- crates/bevy_asset/src/asset_server.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 4c51de3dab288..a5358d7acb655 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -414,19 +414,17 @@ impl AssetServer { if !potential_frees.is_empty() { let asset_lifecycles = self.server.asset_lifecycles.read(); for potential_free in potential_frees { - if let Some(i) = ref_counts.get(&potential_free).cloned() { - if i == 0 { - let type_uuid = match potential_free { - HandleId::Id(type_uuid, _) => Some(type_uuid), - HandleId::AssetPathId(id) => asset_sources - .get(&id.source_path_id()) - .and_then(|source_info| source_info.get_asset_type(id.label_id())), - }; - - if let Some(type_uuid) = type_uuid { - if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) { - asset_lifecycle.free_asset(potential_free); - } + if let Some(&0) = ref_counts.get(&potential_free) { + let type_uuid = match potential_free { + HandleId::Id(type_uuid, _) => Some(type_uuid), + HandleId::AssetPathId(id) => asset_sources + .get(&id.source_path_id()) + .and_then(|source_info| source_info.get_asset_type(id.label_id())), + }; + + if let Some(type_uuid) = type_uuid { + if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) { + asset_lifecycle.free_asset(potential_free); } } } From d024a08a794decbb9ddd63486bff827741d05f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Mon, 26 Apr 2021 01:43:42 +0200 Subject: [PATCH 4/5] separate freeing and marking unused asset, and run free before mark --- crates/bevy_asset/src/asset_server.rs | 52 +++++++++++++++------------ 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index a5358d7acb655..690ac014e4584 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -45,6 +45,7 @@ fn format_missing_asset_ext(exts: &[String]) -> String { pub(crate) struct AssetRefCounter { pub(crate) channel: Arc, pub(crate) ref_counts: Arc>>, + pub(crate) mark_unused_assets: Arc>>, } pub struct AssetServerInternal { @@ -389,10 +390,36 @@ impl AssetServer { } pub fn free_unused_assets(&self) { + let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.write(); + + if !potential_frees.is_empty() { + let ref_counts = self.server.asset_ref_counter.ref_counts.read(); + let asset_sources = self.server.asset_sources.read(); + let asset_lifecycles = self.server.asset_lifecycles.read(); + for potential_free in potential_frees.iter() { + if let Some(&0) = ref_counts.get(potential_free) { + let type_uuid = match potential_free { + HandleId::Id(type_uuid, _) => Some(*type_uuid), + HandleId::AssetPathId(id) => asset_sources + .get(&id.source_path_id()) + .and_then(|source_info| source_info.get_asset_type(id.label_id())), + }; + + if let Some(type_uuid) = type_uuid { + if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) { + asset_lifecycle.free_asset(*potential_free); + } + } + } + } + potential_frees.clear(); + } + } + + pub fn mark_unused_assets(&self) { let receiver = &self.server.asset_ref_counter.channel.receiver; let mut ref_counts = self.server.asset_ref_counter.ref_counts.write(); - let asset_sources = self.server.asset_sources.read(); - let mut potential_frees = Vec::new(); + let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.write(); loop { let ref_change = match receiver.try_recv() { Ok(ref_change) => ref_change, @@ -410,26 +437,6 @@ impl AssetServer { } } } - - if !potential_frees.is_empty() { - let asset_lifecycles = self.server.asset_lifecycles.read(); - for potential_free in potential_frees { - if let Some(&0) = ref_counts.get(&potential_free) { - let type_uuid = match potential_free { - HandleId::Id(type_uuid, _) => Some(type_uuid), - HandleId::AssetPathId(id) => asset_sources - .get(&id.source_path_id()) - .and_then(|source_info| source_info.get_asset_type(id.label_id())), - }; - - if let Some(type_uuid) = type_uuid { - if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) { - asset_lifecycle.free_asset(potential_free); - } - } - } - } - } } fn create_assets_in_load_context(&self, load_context: &mut LoadContext) { @@ -506,6 +513,7 @@ impl AssetServer { pub fn free_unused_assets_system(asset_server: Res) { asset_server.free_unused_assets(); + asset_server.mark_unused_assets(); } #[cfg(test)] From c51d2317133c84b2527da0bee40bdd9e41fda5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Wed, 28 Apr 2021 00:23:36 +0200 Subject: [PATCH 5/5] apply review Co-Authored-By: Nathan Ward <43621845+NathanSWard@users.noreply.github.com> --- crates/bevy_asset/src/asset_server.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 690ac014e4584..4c03a124f4e23 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -10,7 +10,7 @@ use bevy_log::warn; use bevy_tasks::TaskPool; use bevy_utils::{HashMap, Uuid}; use crossbeam_channel::TryRecvError; -use parking_lot::RwLock; +use parking_lot::{Mutex, RwLock}; use std::{collections::hash_map::Entry, path::Path, sync::Arc}; use thiserror::Error; @@ -45,7 +45,7 @@ fn format_missing_asset_ext(exts: &[String]) -> String { pub(crate) struct AssetRefCounter { pub(crate) channel: Arc, pub(crate) ref_counts: Arc>>, - pub(crate) mark_unused_assets: Arc>>, + pub(crate) mark_unused_assets: Arc>>, } pub struct AssetServerInternal { @@ -390,16 +390,16 @@ impl AssetServer { } pub fn free_unused_assets(&self) { - let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.write(); + let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.lock(); if !potential_frees.is_empty() { let ref_counts = self.server.asset_ref_counter.ref_counts.read(); let asset_sources = self.server.asset_sources.read(); let asset_lifecycles = self.server.asset_lifecycles.read(); - for potential_free in potential_frees.iter() { - if let Some(&0) = ref_counts.get(potential_free) { + for potential_free in potential_frees.drain(..) { + if let Some(&0) = ref_counts.get(&potential_free) { let type_uuid = match potential_free { - HandleId::Id(type_uuid, _) => Some(*type_uuid), + HandleId::Id(type_uuid, _) => Some(type_uuid), HandleId::AssetPathId(id) => asset_sources .get(&id.source_path_id()) .and_then(|source_info| source_info.get_asset_type(id.label_id())), @@ -407,19 +407,18 @@ impl AssetServer { if let Some(type_uuid) = type_uuid { if let Some(asset_lifecycle) = asset_lifecycles.get(&type_uuid) { - asset_lifecycle.free_asset(*potential_free); + asset_lifecycle.free_asset(potential_free); } } } } - potential_frees.clear(); } } pub fn mark_unused_assets(&self) { let receiver = &self.server.asset_ref_counter.channel.receiver; let mut ref_counts = self.server.asset_ref_counter.ref_counts.write(); - let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.write(); + let mut potential_frees = None; loop { let ref_change = match receiver.try_recv() { Ok(ref_change) => ref_change, @@ -432,7 +431,11 @@ impl AssetServer { let entry = ref_counts.entry(handle_id).or_insert(0); *entry -= 1; if *entry == 0 { - potential_frees.push(handle_id); + potential_frees + .get_or_insert_with(|| { + self.server.asset_ref_counter.mark_unused_assets.lock() + }) + .push(handle_id); } } }