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.

Sign up for GitHub

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

[Merged by Bors] - Asset re-loading while it's being deleted #2011

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Asset re-loading while it's being deleted #2011

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b70d825
mark `load` and `load_folder` as must_use
mockersf Apr 25, 2021
5a83a0c
return fast if asset is still loading
mockersf Apr 25, 2021
6c260e0
remove unneeded if level
mockersf Apr 25, 2021
d024a08
separate freeing and marking unused asset, and run free before mark
mockersf Apr 25, 2021
c51d231
apply review
mockersf Apr 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 43 additions & 29 deletions crates/bevy_asset/src/asset_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -45,6 +45,7 @@ fn format_missing_asset_ext(exts: &[String]) -> String {
pub(crate) struct AssetRefCounter {
pub(crate) channel: Arc<RefChangeChannel>,
pub(crate) ref_counts: Arc<RwLock<HashMap<HandleId, usize>>>,
pub(crate) mark_unused_assets: Arc<Mutex<Vec<HandleId>>>,
}

pub struct AssetServerInternal {
Expand Down Expand Up @@ -224,6 +225,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<AssetPath<'a>>>(&self, path: P) -> Handle<T> {
self.load_untyped(path).typed()
}
Expand Down Expand Up @@ -253,11 +255,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);
}
Expand Down Expand Up @@ -324,7 +327,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());
}
}

Expand All @@ -336,6 +340,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<AssetPath<'a>>>(&self, path: P) -> HandleUntyped {
let handle_id = self.load_untracked(path.into(), false);
self.get_handle_untyped(handle_id)
Expand All @@ -355,6 +360,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<P: AsRef<Path>>(
&self,
path: P,
Expand Down Expand Up @@ -384,10 +390,35 @@ impl AssetServer {
}

pub fn free_unused_assets(&self) {
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.drain(..) {
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);
}
}
}
}
}
}

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 = None;
loop {
let ref_change = match receiver.try_recv() {
Ok(ref_change) => ref_change,
Expand All @@ -400,29 +431,11 @@ impl AssetServer {
let entry = ref_counts.entry(handle_id).or_insert(0);
*entry -= 1;
if *entry == 0 {
potential_frees.push(handle_id);
}
}
}
}

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);
}
}
potential_frees
.get_or_insert_with(|| {
self.server.asset_ref_counter.mark_unused_assets.lock()
})
.push(handle_id);
}
}
}
Expand Down Expand Up @@ -503,6 +516,7 @@ impl AssetServer {

pub fn free_unused_assets_system(asset_server: Res<AssetServer>) {
asset_server.free_unused_assets();
asset_server.mark_unused_assets();
}

#[cfg(test)]
Expand Down