From 1b2f2965e387ded46ace15c3b6d61336381d60ae Mon Sep 17 00:00:00 2001 From: andriyDev Date: Sat, 2 Aug 2025 23:40:45 -0700 Subject: [PATCH 1/2] Add a test to check that we're only launching one load task when loading subassets. --- crates/bevy_asset/src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 3c939b07cf8b0..3fe8c2b60f123 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -2087,4 +2087,82 @@ mod tests { Some(()) }); } + + #[test] + fn loading_subassets_in_parallel_results_in_one_task() { + struct TestAssetLoader; + + impl AssetLoader for TestAssetLoader { + type Asset = TestAsset; + type Error = std::io::Error; + type Settings = (); + + async fn load( + &self, + _reader: &mut dyn Reader, + _settings: &Self::Settings, + load_context: &mut LoadContext<'_>, + ) -> core::result::Result { + load_context.add_labeled_asset("A".into(), SubText { text: "A".into() }); + load_context.add_labeled_asset("B".into(), SubText { text: "B".into() }); + + Ok(TestAsset) + } + + fn extensions(&self) -> &[&str] { + &["ron"] + } + } + + let dir = Dir::default(); + dir.insert_asset(Path::new("test.ron"), &[]); + + let asset_source = AssetSource::build() + .with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() })); + + // Set up the app. + + let mut app = App::new(); + + app.register_asset_source(AssetSourceId::Default, asset_source) + .add_plugins((TaskPoolPlugin::default(), AssetPlugin::default())) + .init_asset::() + .init_asset::() + .register_asset_loader(TestAssetLoader); + + let asset_server = app.world().resource::(); + let _handle_a: Handle = asset_server.load("test.ron#A"); + let _handle_b: Handle = asset_server.load("test.ron#B"); + + /// A function to take the asset events out of the world so they aren't deleted. + fn take_events(app: &mut App, events: &mut Vec>) { + events.extend( + app.world_mut() + .resource_mut::>>() + .drain(), + ); + } + + // We have no API for determining whether two tasks were spawned (that's not exposed by the + // asset server or even the TaskPool API). So the best we can do is just stall for a bit to + // give both (hypothetical) tasks time to finish. If we were to break this feature, this + // test would become flaky. + let mut events = vec![]; + for _ in 0..5 { + app.update(); + take_events(&mut app, &mut events); + } + + let mut received_adds = 0; + for event in events { + match event { + AssetEvent::Added { .. } => { received_adds += 1; } + AssetEvent::LoadedWithDependencies { .. } => {} + AssetEvent::Modified { .. } => panic!("None of the assets should have been modified, since there should only be one load going on"), + _ => {} + } + } + + assert_eq!(received_adds, 2); + } } From 4560b430643e4db38ff1f0dd7b40122ec134801a Mon Sep 17 00:00:00 2001 From: andriyDev Date: Wed, 6 Aug 2025 23:59:52 -0700 Subject: [PATCH 2/2] Create a "loading guard" to hold a count to prevent future loads from loading the root asset. --- crates/bevy_asset/src/server/info.rs | 1 + crates/bevy_asset/src/server/mod.rs | 59 +++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index 1b3bb3cb65d68..6f2cd4917f4eb 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -85,6 +85,7 @@ pub(crate) struct AssetInfos { pub(crate) dependency_loaded_event_sender: TypeIdMap, pub(crate) dependency_failed_event_sender: TypeIdMap, AssetLoadError)>, + pub(crate) root_asset_to_loading_count: HashMap<(TypeId, AssetPath<'static>), u32>, pub(crate) pending_tasks: HashMap>, } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 69dc8428da87d..14cdf6f062630 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -26,8 +26,9 @@ use alloc::{ }; use atomicow::CowArc; use bevy_ecs::prelude::*; -use bevy_platform::collections::HashSet; +use bevy_platform::collections::{hash_map::Entry, HashSet}; use bevy_tasks::IoTaskPool; +use bevy_utils::OnDrop; use core::{any::TypeId, future::Future, panic::AssertUnwindSafe, task::Poll}; use crossbeam_channel::{Receiver, Sender}; use either::Either; @@ -677,6 +678,62 @@ impl AssetServer { } })?; + // We want to prevent loading the same root asset multiple times + // (`get_or_create_path_handle` prevents loading an asset again if the asset is already + // loaded). So flag that the root asset is loading when we start loading the asset, and + // clear that flag when the asset stops loading. + let _loading_guard = { + let asset_type_id = loader.asset_type_id(); + let root_path = path.without_label().clone_owned(); + + let mut infos = self.data.infos.write(); + let entry = infos + .root_asset_to_loading_count + .entry((asset_type_id, root_path.clone_owned())); + + match (entry, input_handle.is_some()) { + (Entry::Occupied(_), true) => { + // Bail out early. There's already a load for this asset running, and we already + // have the handle we need. + return Ok(None); + } + (Entry::Occupied(mut entry), false) => { + // There is already a load running, but the caller expects us to return an asset + // handle (or an error). So we proceed with the load anyway and just count this + // new load. For a concrete case, if we use `load_untyped_async`, we need to get + // the handle back, so we can't just bail out with no handle - we need to go + // through the rest of the loading process. + *entry.get_mut() += 1; + } + (Entry::Vacant(entry), _) => { + // There's a new load, so keep track of it. + entry.insert(1); + } + } + + // On drop, we should decrement the loading count so future loads can load the asset. + let this = self; + OnDrop::new(move || { + let mut infos = this.data.infos.write(); + let Entry::Occupied(mut entry) = infos + .root_asset_to_loading_count + .entry((asset_type_id, root_path)) + else { + // Each load increments the loading count before starting, and we only decrement + // the count in this OnDrop (at most once per load), so this should never be + // missing. + unreachable!("No loading count associated with currently loading asset."); + }; + + // Decrement the loading count and remove it if the count is zero (so the entry is + // no longer occupied). + *entry.get_mut() -= 1; + if *entry.get() == 0 { + entry.remove(); + } + }) + }; + if let Some(meta_transform) = input_handle.as_ref().and_then(|h| h.meta_transform()) { (*meta_transform)(&mut *meta); }