From 7498cab1cc6309ae8c8cdc04a8d5bccb1b4acf5e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Fri, 8 Sep 2023 13:32:06 -0700 Subject: [PATCH 01/12] Copy on Write AssetPaths --- crates/bevy_asset/src/handle.rs | 2 +- crates/bevy_asset/src/io/processor_gated.rs | 2 +- crates/bevy_asset/src/loader.rs | 86 +++++------ crates/bevy_asset/src/path.rs | 151 +++++++++++++------- crates/bevy_asset/src/processor/mod.rs | 24 ++-- crates/bevy_asset/src/saver.rs | 20 ++- crates/bevy_asset/src/server/info.rs | 5 +- crates/bevy_asset/src/server/mod.rs | 86 ++++++----- crates/bevy_utils/src/cow_arc.rs | 100 +++++++++++++ crates/bevy_utils/src/lib.rs | 2 + examples/shader/texture_binding_array.rs | 5 +- examples/tools/scene_viewer/main.rs | 2 +- 12 files changed, 324 insertions(+), 161 deletions(-) create mode 100644 crates/bevy_utils/src/cow_arc.rs diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index 81de4456b4a78..9e8764d3624b4 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -274,7 +274,7 @@ impl UntypedHandle { /// Returns the path if this is (1) a strong handle and (2) the asset has a path #[inline] - pub fn path(&self) -> Option<&AssetPath<'static>> { + pub fn path(&self) -> Option<&AssetPath> { match self { UntypedHandle::Strong(handle) => handle.path.as_ref(), UntypedHandle::Weak(_) => None, diff --git a/crates/bevy_asset/src/io/processor_gated.rs b/crates/bevy_asset/src/io/processor_gated.rs index fcf891a8f7174..26b20e8d31e95 100644 --- a/crates/bevy_asset/src/io/processor_gated.rs +++ b/crates/bevy_asset/src/io/processor_gated.rs @@ -36,7 +36,7 @@ impl ProcessorGatedReader { ) -> Result, AssetReaderError> { let infos = self.processor_data.asset_infos.read().await; let info = infos - .get(&AssetPath::new(path.to_owned(), None)) + .get(&AssetPath::new(path.to_path_buf())) .ok_or_else(|| AssetReaderError::NotFound(path.to_owned()))?; Ok(info.file_transaction_lock.read_arc().await) } diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 261ef22a08d1a..71fb8dfaf5629 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -8,7 +8,7 @@ use crate::{ Asset, AssetLoadError, AssetServer, Assets, Handle, UntypedAssetId, UntypedHandle, }; use bevy_ecs::world::World; -use bevy_utils::{BoxedFuture, HashMap, HashSet}; +use bevy_utils::{BoxedFuture, CowArc, HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; use futures_lite::AsyncReadExt; use ron::error::SpannedError; @@ -143,7 +143,7 @@ pub struct LoadedAsset { pub(crate) value: A, pub(crate) dependencies: HashSet, pub(crate) loader_dependencies: HashMap, AssetHash>, - pub(crate) labeled_assets: HashMap, + pub(crate) labeled_assets: HashMap, LabeledAsset>, pub(crate) meta: Option>, } @@ -175,7 +175,7 @@ pub struct ErasedLoadedAsset { pub(crate) value: Box, pub(crate) dependencies: HashSet, pub(crate) loader_dependencies: HashMap, AssetHash>, - pub(crate) labeled_assets: HashMap, + pub(crate) labeled_assets: HashMap, LabeledAsset>, pub(crate) meta: Option>, } @@ -214,13 +214,16 @@ impl ErasedLoadedAsset { } /// Returns the [`ErasedLoadedAsset`] for the given label, if it exists. - pub fn get_labeled(&self, label: &str) -> Option<&ErasedLoadedAsset> { - self.labeled_assets.get(label).map(|a| &a.asset) + pub fn get_labeled( + &self, + label: impl Into>, + ) -> Option<&ErasedLoadedAsset> { + self.labeled_assets.get(&label.into()).map(|a| &a.asset) } /// Iterate over all labels for "labeled assets" in the loaded asset pub fn iter_labels(&self) -> impl Iterator { - self.labeled_assets.keys().map(|s| s.as_str()) + self.labeled_assets.keys().map(|s| &**s) } } @@ -269,7 +272,7 @@ pub struct LoadContext<'a> { dependencies: HashSet, /// Direct dependencies used by this loader. loader_dependencies: HashMap, AssetHash>, - labeled_assets: HashMap, + labeled_assets: HashMap, LabeledAsset>, } impl<'a> LoadContext<'a> { @@ -362,9 +365,10 @@ impl<'a> LoadContext<'a> { /// See [`AssetPath`] for more on labeled assets. pub fn add_loaded_labeled_asset( &mut self, - label: String, + label: impl Into>, loaded_asset: LoadedAsset, ) -> Handle { + let label = label.into(); let loaded_asset: ErasedLoadedAsset = loaded_asset.into(); let labeled_path = self.asset_path.with_label(label.clone()); let handle = self @@ -383,9 +387,9 @@ impl<'a> LoadContext<'a> { /// Returns `true` if an asset with the label `label` exists in this context. /// /// See [`AssetPath`] for more on labeled assets. - pub fn has_labeled_asset(&self, label: &str) -> bool { - let path = self.asset_path.with_label(label); - self.asset_server.get_handle_untyped(path).is_some() + pub fn has_labeled_asset<'b>(&self, label: impl Into>) -> bool { + let path = self.asset_path.with_label(label.into()); + self.asset_server.get_handle_untyped(&path).is_some() } /// "Finishes" this context by populating the final [`Asset`] value (and the erased [`AssetMeta`] value, if it exists). @@ -406,7 +410,7 @@ impl<'a> LoadContext<'a> { } /// Gets the source asset path for this load context. - pub fn asset_path(&self) -> &AssetPath { + pub fn asset_path(&self) -> &AssetPath<'static> { &self.asset_path } @@ -432,7 +436,7 @@ impl<'a> LoadContext<'a> { let mut bytes = Vec::new(); reader.read_to_end(&mut bytes).await?; self.loader_dependencies - .insert(AssetPath::new(path.to_owned(), None), hash); + .insert(AssetPath::new(path.to_owned()), hash); Ok(bytes) } @@ -461,14 +465,12 @@ impl<'a> LoadContext<'a> { path: impl Into>, settings: impl Fn(&mut S) + Send + Sync + 'static, ) -> Handle { - let path = path.into().to_owned(); + let path = path.into(); let handle = if self.should_load_dependencies { self.asset_server.load_with_settings(path.clone(), settings) } else { - self.asset_server.get_or_create_path_handle( - path.clone(), - Some(loader_settings_meta_transform(settings)), - ) + self.asset_server + .get_or_create_path_handle(path, Some(loader_settings_meta_transform(settings))) }; self.dependencies.insert(handle.id().untyped()); handle @@ -477,8 +479,11 @@ impl<'a> LoadContext<'a> { /// Returns a handle to an asset of type `A` with the label `label`. This [`LoadContext`] must produce an asset of the /// given type and the given label or the dependencies of this asset will never be considered "fully loaded". However you /// can call this method before _or_ after adding the labeled asset. - pub fn get_label_handle(&mut self, label: &str) -> Handle { - let path = self.asset_path.with_label(label).to_owned(); + pub fn get_label_handle<'b, A: Asset>( + &mut self, + label: impl Into>, + ) -> Handle { + let path = self.asset_path.with_label(label); let handle = self.asset_server.get_or_create_path_handle::(path, None); self.dependencies.insert(handle.id().untyped()); handle @@ -498,36 +503,37 @@ impl<'a> LoadContext<'a> { &mut self, path: impl Into>, ) -> Result { - let path = path.into(); + let path = path.into().into_owned(); let to_error = |e: AssetLoadError| -> LoadDirectError { LoadDirectError { - dependency: path.to_owned(), + dependency: path.clone(), error: e, } }; - let (meta, loader, mut reader) = self - .asset_server - .get_meta_loader_and_reader(&path) - .await - .map_err(to_error)?; - let loaded_asset = self - .asset_server - .load_with_meta_loader_and_reader( - &path, - meta, - &*loader, - &mut *reader, - false, - self.populate_hashes, - ) - .await - .map_err(to_error)?; + let loaded_asset = { + let (meta, loader, mut reader) = self + .asset_server + .get_meta_loader_and_reader(&path) + .await + .map_err(to_error)?; + self.asset_server + .load_with_meta_loader_and_reader( + &path, + meta, + &*loader, + &mut *reader, + false, + self.populate_hashes, + ) + .await + .map_err(to_error)? + }; let info = loaded_asset .meta .as_ref() .and_then(|m| m.processed_info().as_ref()); let hash = info.map(|i| i.full_hash).unwrap_or(Default::default()); - self.loader_dependencies.insert(path.to_owned(), hash); + self.loader_dependencies.insert(path, hash); Ok(loaded_asset) } } diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 108371e8589ca..df53a50a2b980 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -1,9 +1,10 @@ use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; -use serde::{Deserialize, Serialize}; +use bevy_utils::CowArc; +use serde::{de::Visitor, ser::SerializeTupleStruct, Deserialize, Serialize}; use std::{ - borrow::Cow, fmt::{Debug, Display}, hash::Hash, + ops::Deref, path::{Path, PathBuf}, }; @@ -33,11 +34,11 @@ use std::{ /// // This loads the `PlayerMesh` labeled asset from the `my_scene.scn` base asset. /// let mesh: Handle = asset_server.load("my_scene.scn#PlayerMesh"); /// ``` -#[derive(Eq, PartialEq, Hash, Clone, Serialize, Deserialize, Reflect)] +#[derive(Eq, PartialEq, Hash, Clone, Reflect)] #[reflect(Debug, PartialEq, Hash, Serialize, Deserialize)] pub struct AssetPath<'a> { - pub path: Cow<'a, Path>, - pub label: Option>, + path: CowArc<'a, Path>, + label: Option>, } impl<'a> Debug for AssetPath<'a> { @@ -48,9 +49,9 @@ impl<'a> Debug for AssetPath<'a> { impl<'a> Display for AssetPath<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.path.display())?; + write!(f, "{:?}", &*self.path)?; if let Some(label) = &self.label { - write!(f, "#{label}")?; + write!(f, "#{}", &**label)?; } Ok(()) } @@ -59,72 +60,69 @@ impl<'a> Display for AssetPath<'a> { impl<'a> AssetPath<'a> { /// Creates a new asset path using borrowed information. #[inline] - pub fn new_ref(path: &'a Path, label: Option<&'a str>) -> AssetPath<'a> { + pub fn new(path: impl Into>) -> AssetPath<'a> { AssetPath { - path: Cow::Borrowed(path), - label: label.map(Cow::Borrowed), - } - } - - /// Creates a new asset path. - #[inline] - pub fn new(path: PathBuf, label: Option) -> AssetPath<'a> { - AssetPath { - path: Cow::Owned(path), - label: label.map(Cow::Owned), + path: path.into(), + label: None, } } /// Gets the "sub-asset label". #[inline] pub fn label(&self) -> Option<&str> { - self.label.as_ref().map(|label| label.as_ref()) + self.label.as_deref() } /// Gets the path to the asset in the "virtual filesystem". #[inline] pub fn path(&self) -> &Path { - &self.path + self.path.deref() } /// Gets the path to the asset in the "virtual filesystem" without a label (if a label is currently set). #[inline] pub fn without_label(&self) -> AssetPath<'_> { - AssetPath::new_ref(&self.path, None) + Self { + path: self.path.clone(), + label: None, + } + } + + /// Removes a "sub-asset label" from this [`AssetPath`], if one was set. + #[inline] + pub fn remove_label(&mut self) { + self.label = None; } - /// Removes a "sub-asset label" from this [`AssetPath`] and returns it, if one was set. + /// Takes the "sub-asset label" from this [`AssetPath`], if one was set. #[inline] - pub fn remove_label(&mut self) -> Option> { + pub fn take_label(&mut self) -> Option> { self.label.take() } /// Returns this asset path with the given label. This will replace the previous /// label if it exists. #[inline] - pub fn with_label(&self, label: impl Into>) -> AssetPath<'a> { + pub fn with_label(&self, label: impl Into>) -> AssetPath<'a> { AssetPath { path: self.path.clone(), label: Some(label.into()), } } - /// Converts the borrowed path data to owned. - #[inline] - pub fn to_owned(&self) -> AssetPath<'static> { + /// Converts this into an "owned" value. If internally a value is borrowed, it will be cloned into an "owned [`Arc`]". + /// If it is already an "owned [`Arc`]", it will remain unchanged. + pub fn into_owned(self) -> AssetPath<'static> { AssetPath { - path: Cow::Owned(self.path.to_path_buf()), - label: self - .label - .as_ref() - .map(|value| Cow::Owned(value.to_string())), + path: self.path.into_owned(), + label: self.label.map(|l| l.into_owned()), } } /// Returns the full extension (including multiple '.' values). /// Ex: Returns `"config.ron"` for `"my_asset.config.ron"` pub fn get_full_extension(&self) -> Option { - let file_name = self.path.file_name()?.to_str()?; + let file_name = self.path().file_name()?.to_str()?; let index = file_name.find('.')?; let extension = file_name[index + 1..].to_lowercase(); Some(extension) @@ -146,42 +144,99 @@ impl<'a> From<&'a str> for AssetPath<'a> { let mut parts = asset_path.splitn(2, '#'); let path = Path::new(parts.next().expect("Path must be set.")); let label = parts.next(); - AssetPath { - path: Cow::Borrowed(path), - label: label.map(Cow::Borrowed), + Self { + path: path.into(), + label: label.map(|l| l.into()), } } } - impl<'a> From<&'a String> for AssetPath<'a> { + #[inline] fn from(asset_path: &'a String) -> Self { asset_path.as_str().into() } } +impl From for AssetPath<'static> { + #[inline] + fn from(asset_path: String) -> Self { + AssetPath::from(asset_path.as_str()).into_owned() + } +} + impl<'a> From<&'a Path> for AssetPath<'a> { + #[inline] fn from(path: &'a Path) -> Self { - AssetPath { - path: Cow::Borrowed(path), + Self { + path: path.into(), label: None, } } } -impl<'a> From for AssetPath<'a> { +impl From for AssetPath<'static> { + #[inline] fn from(path: PathBuf) -> Self { - AssetPath { - path: Cow::Owned(path), + Self { + path: path.into(), label: None, } } } +impl<'a, 'b> From<&'a AssetPath<'b>> for AssetPath<'b> { + fn from(value: &'a AssetPath<'b>) -> Self { + value.clone() + } +} + impl<'a> From> for PathBuf { - fn from(path: AssetPath<'a>) -> Self { - match path.path { - Cow::Borrowed(borrowed) => borrowed.to_owned(), - Cow::Owned(owned) => owned, - } + fn from(value: AssetPath<'a>) -> Self { + value.path().to_path_buf() + } +} + +impl<'a> Serialize for AssetPath<'a> { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let mut state = serializer.serialize_tuple_struct("AssetPath", 1)?; + let string = self.to_string(); + state.serialize_field(&string)?; + state.end() + } +} + +impl<'de> Deserialize<'de> for AssetPath<'static> { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_tuple_struct("AssetPath", 1, AssetPathVisitor) + } +} + +struct AssetPathVisitor; + +impl<'de> Visitor<'de> for AssetPathVisitor { + type Value = AssetPath<'static>; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("string AssetPath") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + Ok(AssetPath::from(v).into_owned()) + } + + fn visit_string(self, v: String) -> Result + where + E: serde::de::Error, + { + Ok(AssetPath::from(v)) } } diff --git a/crates/bevy_asset/src/processor/mod.rs b/crates/bevy_asset/src/processor/mod.rs index b1a9f84ccdf63..a537063598377 100644 --- a/crates/bevy_asset/src/processor/mod.rs +++ b/crates/bevy_asset/src/processor/mod.rs @@ -364,7 +364,7 @@ impl AssetProcessor { /// asset have finished, thanks to the `file_transaction_lock`. async fn handle_removed_asset(&self, path: PathBuf) { debug!("Removing processed {:?} because source was removed", path); - let asset_path = AssetPath::new(path, None); + let asset_path = AssetPath::new(path); let mut infos = self.data.asset_infos.write().await; if let Some(info) = infos.get(&asset_path) { // we must wait for uncontested write access to the asset source to ensure existing readers / writers @@ -380,19 +380,19 @@ impl AssetProcessor { /// This will cause direct path dependencies to break. async fn handle_renamed_asset(&self, old: PathBuf, new: PathBuf) { let mut infos = self.data.asset_infos.write().await; - let old_asset_path = AssetPath::new(old, None); + let old_asset_path = AssetPath::new(old); if let Some(info) = infos.get(&old_asset_path) { // we must wait for uncontested write access to the asset source to ensure existing readers / writers // can finish their operations let _write_lock = info.file_transaction_lock.write(); - let old = &old_asset_path.path; + let old = old_asset_path.path(); self.destination_writer().rename(old, &new).await.unwrap(); self.destination_writer() .rename_meta(old, &new) .await .unwrap(); } - let new_asset_path = AssetPath::new(new.clone(), None); + let new_asset_path = AssetPath::new(new); infos.rename(&old_asset_path, &new_asset_path).await; } @@ -531,11 +531,11 @@ impl AssetProcessor { .map_err(InitializeError::FailedToReadDestinationPaths)?; for path in &source_paths { - asset_infos.get_or_insert(AssetPath::new(path.to_owned(), None)); + asset_infos.get_or_insert(AssetPath::new(path.clone())); } for path in &destination_paths { - let asset_path = AssetPath::new(path.to_owned(), None); + let asset_path = AssetPath::new(path.clone()); let mut dependencies = Vec::new(); if let Some(info) = asset_infos.get_mut(&asset_path) { match self.destination_reader().read_meta_bytes(path).await { @@ -551,7 +551,7 @@ impl AssetProcessor { for process_dependency_info in &processed_info.process_dependencies { - dependencies.push(process_dependency_info.path.to_owned()); + dependencies.push(process_dependency_info.path.clone()); } } info.processed_info = minimal.processed_info; @@ -573,7 +573,7 @@ impl AssetProcessor { } for dependency in dependencies { - asset_infos.add_dependant(&dependency, asset_path.to_owned()); + asset_infos.add_dependant(&dependency, asset_path.clone()); } } @@ -627,7 +627,7 @@ impl AssetProcessor { async fn process_asset(&self, path: &Path) { let result = self.process_asset_internal(path).await; let mut infos = self.data.asset_infos.write().await; - let asset_path = AssetPath::new(path.to_owned(), None); + let asset_path = AssetPath::new(path.to_owned()); infos.finish_processing(asset_path, result).await; } @@ -635,7 +635,7 @@ impl AssetProcessor { if path.extension().is_none() { return Err(ProcessError::ExtensionRequired); } - let asset_path = AssetPath::new(path.to_owned(), None); + let asset_path = AssetPath::new(path.to_path_buf()); // TODO: check if already processing to protect against duplicate hot-reload events debug!("Processing {:?}", path); let server = &self.server; @@ -912,7 +912,7 @@ impl AssetProcessorData { self.wait_until_initialized().await; let mut receiver = { let infos = self.asset_infos.write().await; - let info = infos.get(&AssetPath::new(path.to_owned(), None)); + let info = infos.get(&AssetPath::new(path.to_path_buf())); match info { Some(info) => match info.status { Some(result) => return result, @@ -1067,7 +1067,7 @@ impl ProcessorAssetInfos { } else { let dependants = self .non_existent_dependants - .entry(asset_path.to_owned()) + .entry(asset_path.clone()) .or_default(); dependants.insert(dependant); } diff --git a/crates/bevy_asset/src/saver.rs b/crates/bevy_asset/src/saver.rs index 62cfcc0c59abe..16d9007419c28 100644 --- a/crates/bevy_asset/src/saver.rs +++ b/crates/bevy_asset/src/saver.rs @@ -1,6 +1,6 @@ use crate::{io::Writer, meta::Settings, Asset, ErasedLoadedAsset}; use crate::{AssetLoader, LabeledAsset}; -use bevy_utils::{BoxedFuture, HashMap}; +use bevy_utils::{BoxedFuture, CowArc, HashMap}; use serde::{Deserialize, Serialize}; use std::ops::Deref; @@ -60,7 +60,7 @@ impl ErasedAssetSaver for S { /// An [`Asset`] (and any labeled "sub assets") intended to be saved. pub struct SavedAsset<'a, A: Asset> { value: &'a A, - labeled_assets: &'a HashMap, + labeled_assets: &'a HashMap, LabeledAsset>, } impl<'a, A: Asset> Deref for SavedAsset<'a, A> { @@ -88,8 +88,11 @@ impl<'a, A: Asset> SavedAsset<'a, A> { } /// Returns the labeled asset, if it exists and matches this type. - pub fn get_labeled(&self, label: &str) -> Option> { - let labeled = self.labeled_assets.get(label)?; + pub fn get_labeled( + &self, + label: impl Into>, + ) -> Option> { + let labeled = self.labeled_assets.get(&label.into())?; let value = labeled.asset.value.downcast_ref::()?; Some(SavedAsset { value, @@ -98,13 +101,16 @@ impl<'a, A: Asset> SavedAsset<'a, A> { } /// Returns the type-erased labeled asset, if it exists and matches this type. - pub fn get_erased_labeled(&self, label: &str) -> Option<&ErasedLoadedAsset> { - let labeled = self.labeled_assets.get(label)?; + pub fn get_erased_labeled( + &self, + label: impl Into>, + ) -> Option<&ErasedLoadedAsset> { + let labeled = self.labeled_assets.get(&label.into())?; Some(&labeled.asset) } /// Iterate over all labels for "labeled assets" in the loaded asset pub fn iter_labels(&self) -> impl Iterator { - self.labeled_assets.keys().map(|s| s.as_str()) + self.labeled_assets.keys().map(|s| &**s) } } diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index 83147c63a0cbe..baa4007001e95 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -257,8 +257,9 @@ impl AssetInfos { } /// Returns `true` if this path has - pub(crate) fn is_path_alive(&self, path: &AssetPath) -> bool { - if let Some(id) = self.path_to_id.get(path) { + pub(crate) fn is_path_alive<'a>(&self, path: impl Into>) -> bool { + let path = path.into(); + if let Some(id) = self.path_to_id.get(&path) { if let Some(info) = self.infos.get(id) { return info.weak_handle.strong_count() > 0; } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 6feda15955e6a..eac1000d20639 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -201,8 +201,9 @@ impl AssetServer { /// Retrieves the default [`AssetLoader`] for the given path, if one can be found. pub async fn get_path_asset_loader<'a>( &self, - path: &AssetPath<'a>, + path: impl Into>, ) -> Result, MissingAssetLoaderForExtensionError> { + let path = path.into(); let full_extension = path.get_full_extension() .ok_or(MissingAssetLoaderForExtensionError { @@ -252,27 +253,22 @@ impl AssetServer { path: impl Into>, meta_transform: Option, ) -> Handle { - let path: AssetPath = path.into(); + let mut path = path.into().into_owned(); let (handle, should_load) = self.data.infos.write().get_or_create_path_handle::( - path.to_owned(), + path.clone(), HandleLoadingMode::Request, meta_transform, ); if should_load { let mut owned_handle = Some(handle.clone().untyped()); - let mut owned_path = path.to_owned(); let server = self.clone(); IoTaskPool::get() .spawn(async move { - if owned_path.label().is_some() { - owned_path.remove_label(); + if path.take_label().is_some() { owned_handle = None; } - if let Err(err) = server - .load_internal(owned_handle, owned_path, false, None) - .await - { + if let Err(err) = server.load_internal(owned_handle, path, false, None).await { error!("{}", err); } }) @@ -287,19 +283,21 @@ impl AssetServer { &self, path: impl Into>, ) -> Result { - self.load_internal(None, path.into(), false, None).await + let path: AssetPath = path.into(); + self.load_internal(None, path, false, None).await } async fn load_internal<'a>( &self, input_handle: Option, - mut path: AssetPath<'a>, + path: AssetPath<'a>, force: bool, meta_transform: Option, ) -> Result { - let owned_path = path.to_owned(); + let mut path = path.into_owned(); + let path_clone = path.clone(); let (mut meta, loader, mut reader) = self - .get_meta_loader_and_reader(&owned_path) + .get_meta_loader_and_reader(&path_clone) .await .map_err(|e| { // if there was an input handle, a "load" operation has already started, so we must produce a "failure" event, if @@ -316,7 +314,7 @@ impl AssetServer { Some(handle) => { if !has_label && handle.type_id() != loader.asset_type_id() { return Err(AssetLoadError::RequestedHandleTypeMismatch { - path: path.to_owned(), + path: path.into_owned(), requested: handle.type_id(), actual_asset_name: loader.asset_type_name(), loader_name: loader.type_name(), @@ -328,7 +326,7 @@ impl AssetServer { None => { let mut infos = self.data.infos.write(); infos.get_or_create_path_handle_untyped( - path.to_owned(), + path.clone(), loader.asset_type_id(), loader.asset_type_name(), HandleLoadingMode::Request, @@ -346,7 +344,7 @@ impl AssetServer { // We need to get the actual asset id let mut infos = self.data.infos.write(); let (actual_handle, _) = infos.get_or_create_path_handle_untyped( - path.to_owned(), + path.clone(), loader.asset_type_id(), loader.asset_type_name(), // ignore current load state ... we kicked off this sub asset load because it needed to be loaded but @@ -390,13 +388,12 @@ impl AssetServer { /// Kicks off a reload of the asset stored at the given path. This will only reload the asset if it currently loaded. pub fn reload<'a>(&self, path: impl Into>) { let server = self.clone(); - let path = path.into(); - let owned_path = path.to_owned(); + let path = path.into().into_owned(); IoTaskPool::get() .spawn(async move { - if server.data.infos.read().is_path_alive(&owned_path) { - info!("Reloading {owned_path} because it has changed"); - if let Err(err) = server.load_internal(None, owned_path, true, None).await { + if server.data.infos.read().is_path_alive(&path) { + info!("Reloading {path} because it has changed"); + if let Err(err) = server.load_internal(None, path, true, None).await { error!("{}", err); } } @@ -423,13 +420,13 @@ impl AssetServer { #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] pub(crate) fn load_asset_untyped( &self, - path: Option<&AssetPath<'static>>, + path: Option>, asset: impl Into, ) -> UntypedHandle { let loaded_asset = asset.into(); let handle = if let Some(path) = path { let (handle, _) = self.data.infos.write().get_or_create_path_handle_untyped( - path.clone(), + path, loaded_asset.asset_type_id(), loaded_asset.asset_type_name(), HandleLoadingMode::NotLoading, @@ -583,10 +580,10 @@ impl AssetServer { } /// Returns the path for the given `id`, if it has one. - pub fn get_path(&self, id: impl Into) -> Option> { + pub fn get_path(&self, id: impl Into) -> Option { let infos = self.data.infos.read(); let info = infos.get(id.into())?; - Some(info.path.as_ref()?.to_owned()) + Some(info.path.as_ref()?.clone()) } /// Pre-register a loader that will later be added. @@ -618,14 +615,18 @@ impl AssetServer { } /// Retrieve a handle for the given path. This will create a handle (and [`AssetInfo`]) if it does not exist - pub(crate) fn get_or_create_path_handle( + pub(crate) fn get_or_create_path_handle<'a, A: Asset>( &self, - path: AssetPath<'static>, + path: impl Into>, meta_transform: Option, ) -> Handle { let mut infos = self.data.infos.write(); infos - .get_or_create_path_handle::(path, HandleLoadingMode::NotLoading, meta_transform) + .get_or_create_path_handle::( + path.into().into_owned(), + HandleLoadingMode::NotLoading, + meta_transform, + ) .0 } @@ -655,19 +656,19 @@ impl AssetServer { AssetActionMinimal::Load { loader } => loader, AssetActionMinimal::Process { .. } => { return Err(AssetLoadError::CannotLoadProcessedAsset { - path: asset_path.to_owned(), + path: asset_path.clone().into_owned(), }) } AssetActionMinimal::Ignore => { return Err(AssetLoadError::CannotLoadIgnoredAsset { - path: asset_path.to_owned(), + path: asset_path.clone().into_owned(), }) } }; let loader = self.get_asset_loader_with_type_name(&loader_name).await?; let meta = loader.deserialize_meta(&meta_bytes).map_err(|e| { AssetLoadError::AssetLoaderError { - path: asset_path.to_owned(), + path: asset_path.clone().into_owned(), loader: loader.type_name(), error: AssetLoaderError::DeserializeMeta(e), } @@ -693,16 +694,14 @@ impl AssetServer { load_dependencies: bool, populate_hashes: bool, ) -> Result { - let load_context = LoadContext::new( - self, - asset_path.to_owned(), - load_dependencies, - populate_hashes, - ); + // TODO: experiment with this + let asset_path = asset_path.clone().into_owned(); + let load_context = + LoadContext::new(self, asset_path.clone(), load_dependencies, populate_hashes); loader.load(reader, meta, load_context).await.map_err(|e| { AssetLoadError::AssetLoaderError { loader: loader.type_name(), - path: asset_path.to_owned(), + path: asset_path, error: e, } }) @@ -753,12 +752,9 @@ pub fn handle_internal_asset_events(world: &mut World) { // TODO: if the asset was processed and the processed file was changed, the first modified event // should be skipped? AssetSourceEvent::ModifiedAsset(path) | AssetSourceEvent::ModifiedMeta(path) => { - queue_ancestors( - &AssetPath::new_ref(&path, None), - &infos, - &mut paths_to_reload, - ); - paths_to_reload.insert(path.into()); + let path = AssetPath::new(path); + queue_ancestors(&path, &infos, &mut paths_to_reload); + paths_to_reload.insert(path); } _ => {} } diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs new file mode 100644 index 0000000000000..c514243baa1ae --- /dev/null +++ b/crates/bevy_utils/src/cow_arc.rs @@ -0,0 +1,100 @@ +use std::{ + fmt::Debug, + hash::Hash, + ops::Deref, + path::{Path, PathBuf}, + sync::Arc, +}; + +/// Much like a [`Cow`](std::borrow::Cow), but owned values are Arc-ed to make clones cheap. This should be used for values that +/// are cloned for use across threads and change rarely (if ever). +pub enum CowArc<'a, T: ?Sized> { + /// A borrowed value + Borrowed(&'a T), + /// An owned [`Arc`]-ed value + Owned(Arc), +} + +impl<'a, T: ?Sized> Deref for CowArc<'a, T> { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + match self { + CowArc::Borrowed(v) => v, + CowArc::Owned(v) => v, + } + } +} + +impl<'a, T: ?Sized> CowArc<'a, T> +where + &'a T: Into>, +{ + /// Converts this into an "owned" value. If internally a value is borrowed, it will be cloned into an "owned [`Arc`]". + /// If it is already an "owned [`Arc`]", it will remain unchanged. + #[inline] + pub fn into_owned(self) -> CowArc<'static, T> { + match self { + CowArc::Borrowed(path) => CowArc::Owned(path.into()), + CowArc::Owned(path) => CowArc::Owned(path), + } + } +} + +impl<'a, T: ?Sized> Clone for CowArc<'a, T> { + fn clone(&self) -> Self { + match self { + Self::Borrowed(value) => Self::Borrowed(value), + Self::Owned(value) => Self::Owned(value.clone()), + } + } +} + +impl<'a, T: PartialEq + ?Sized> PartialEq for CowArc<'a, T> { + fn eq(&self, other: &Self) -> bool { + self.deref().eq(other.deref()) + } +} + +impl<'a, T: PartialEq + ?Sized> Eq for CowArc<'a, T> {} + +impl<'a, T: Hash + ?Sized> Hash for CowArc<'a, T> { + fn hash(&self, state: &mut H) { + self.deref().hash(state); + } +} + +impl From for CowArc<'static, Path> { + #[inline] + fn from(value: PathBuf) -> Self { + CowArc::Owned(value.into()) + } +} + +impl From for CowArc<'static, str> { + #[inline] + fn from(value: String) -> Self { + CowArc::Owned(value.into()) + } +} + +impl<'a> From<&'a String> for CowArc<'a, str> { + #[inline] + fn from(value: &'a String) -> Self { + CowArc::Borrowed(value) + } +} + +impl<'a, T: ?Sized + Debug> Debug for CowArc<'a, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.deref().fmt(f) + } +} + +impl<'a, T: ?Sized> From<&'a T> for CowArc<'a, T> { + #[inline] + fn from(value: &'a T) -> Self { + CowArc::Borrowed(value) + } +} diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 1c3152a043f04..7916caf769476 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -18,11 +18,13 @@ pub use short_names::get_short_name; pub mod synccell; pub mod syncunsafecell; +mod cow_arc; mod default; mod float_ord; pub use ahash::{AHasher, RandomState}; pub use bevy_utils_proc_macros::*; +pub use cow_arc::*; pub use default::default; pub use float_ord::*; pub use hashbrown; diff --git a/examples/shader/texture_binding_array.rs b/examples/shader/texture_binding_array.rs index f395d9fbde9e4..d2302d298c31e 100644 --- a/examples/shader/texture_binding_array.rs +++ b/examples/shader/texture_binding_array.rs @@ -72,10 +72,7 @@ fn setup( // load 16 textures let textures: Vec<_> = TILE_ID .iter() - .map(|id| { - let path = format!("textures/rpg/tiles/generic-rpg-tile{id:0>2}.png"); - asset_server.load(&path) - }) + .map(|id| asset_server.load(format!("textures/rpg/tiles/generic-rpg-tile{id:0>2}.png"))) .collect(); // a cube with multiple textures diff --git a/examples/tools/scene_viewer/main.rs b/examples/tools/scene_viewer/main.rs index 9cdeec38c4eb8..2dbfc97b1c66d 100644 --- a/examples/tools/scene_viewer/main.rs +++ b/examples/tools/scene_viewer/main.rs @@ -77,7 +77,7 @@ fn setup(mut commands: Commands, asset_server: Res) { info!("Loading {}", scene_path); let (file_path, scene_index) = parse_scene(scene_path); - commands.insert_resource(SceneHandle::new(asset_server.load(&file_path), scene_index)); + commands.insert_resource(SceneHandle::new(asset_server.load(file_path), scene_index)); } fn setup_scene_after_load( From ab60ba9a9b9e1a8877f72ecdbc5084bbdc0d4906 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Fri, 8 Sep 2023 18:15:46 -0700 Subject: [PATCH 02/12] Fix doc --- crates/bevy_asset/src/path.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index df53a50a2b980..3d6eac243db41 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -112,6 +112,8 @@ impl<'a> AssetPath<'a> { /// Converts this into an "owned" value. If internally a value is borrowed, it will be cloned into an "owned [`Arc`]". /// If it is already an "owned [`Arc`]", it will remain unchanged. + /// + /// [`Arc`]: std::sync::Arc pub fn into_owned(self) -> AssetPath<'static> { AssetPath { path: self.path.into_owned(), From d21296016e338649b27ec48f642821a94f8e1c3a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Fri, 8 Sep 2023 18:17:25 -0700 Subject: [PATCH 03/12] Keep "staticness" of path --- crates/bevy_asset/src/handle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index 9e8764d3624b4..81de4456b4a78 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -274,7 +274,7 @@ impl UntypedHandle { /// Returns the path if this is (1) a strong handle and (2) the asset has a path #[inline] - pub fn path(&self) -> Option<&AssetPath> { + pub fn path(&self) -> Option<&AssetPath<'static>> { match self { UntypedHandle::Strong(handle) => handle.path.as_ref(), UntypedHandle::Weak(_) => None, From f9507457e4c1b21fa1c44d9edbba66f1f4c0a381 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 11:09:43 -0700 Subject: [PATCH 04/12] Update crates/bevy_utils/src/cow_arc.rs Co-authored-by: Pascal Hertleif --- crates/bevy_utils/src/cow_arc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index c514243baa1ae..30cd44d90cea5 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -36,8 +36,8 @@ where #[inline] pub fn into_owned(self) -> CowArc<'static, T> { match self { - CowArc::Borrowed(path) => CowArc::Owned(path.into()), - CowArc::Owned(path) => CowArc::Owned(path), + CowArc::Borrowed(value) => CowArc::Owned(value.into()), + CowArc::Owned(value) => CowArc::Owned(value), } } } From 73b4549fb8ecc62e8d646afe6caee9973a2f222b Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 11:23:15 -0700 Subject: [PATCH 05/12] Impl Display and Debug for CowArc. Improve assetpath display impl --- crates/bevy_asset/src/path.rs | 4 ++-- crates/bevy_utils/src/cow_arc.rs | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 3d6eac243db41..514bddafd1c01 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -49,9 +49,9 @@ impl<'a> Debug for AssetPath<'a> { impl<'a> Display for AssetPath<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", &*self.path)?; + write!(f, "{}", self.path.display())?; if let Some(label) = &self.label { - write!(f, "#{}", &**label)?; + write!(f, "#{label}")?; } Ok(()) } diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index 30cd44d90cea5..e222f8abfff2c 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -1,5 +1,5 @@ use std::{ - fmt::Debug, + fmt::{Debug, Display}, hash::Hash, ops::Deref, path::{Path, PathBuf}, @@ -65,6 +65,18 @@ impl<'a, T: Hash + ?Sized> Hash for CowArc<'a, T> { } } +impl<'a, T: Debug + ?Sized> Debug for CowArc<'a, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + Debug::fmt(self.deref(), f) + } +} + +impl<'a, T: Display + ?Sized> Display for CowArc<'a, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + Display::fmt(self.deref(), f) + } +} + impl From for CowArc<'static, Path> { #[inline] fn from(value: PathBuf) -> Self { @@ -72,6 +84,13 @@ impl From for CowArc<'static, Path> { } } +impl<'a> From<&'a str> for CowArc<'a, Path> { + #[inline] + fn from(value: &'a str) -> Self { + CowArc::Borrowed(Path::new(value)) + } +} + impl From for CowArc<'static, str> { #[inline] fn from(value: String) -> Self { @@ -86,12 +105,6 @@ impl<'a> From<&'a String> for CowArc<'a, str> { } } -impl<'a, T: ?Sized + Debug> Debug for CowArc<'a, T> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.deref().fmt(f) - } -} - impl<'a, T: ?Sized> From<&'a T> for CowArc<'a, T> { #[inline] fn from(value: &'a T) -> Self { From 77f6f316474ae4964a5076014b37b3000e295db9 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 11:25:58 -0700 Subject: [PATCH 06/12] Add Ord/PartialOrd --- crates/bevy_utils/src/cow_arc.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index e222f8abfff2c..a555a4e53acad 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -77,6 +77,18 @@ impl<'a, T: Display + ?Sized> Display for CowArc<'a, T> { } } +impl<'a, T: PartialOrd + ?Sized> PartialOrd for CowArc<'a, T> { + fn partial_cmp(&self, other: &Self) -> Option { + self.deref().partial_cmp(other.deref()) + } +} + +impl<'a, T: Ord + ?Sized> Ord for CowArc<'a, T> { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.deref().cmp(other.deref()) + } +} + impl From for CowArc<'static, Path> { #[inline] fn from(value: PathBuf) -> Self { From 5afb4c97a91610551c45d8083c4900dee074a3dd Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 12:09:45 -0700 Subject: [PATCH 07/12] Capture and retain static state by default in CowArc --- crates/bevy_asset/src/io/processor_gated.rs | 2 +- crates/bevy_asset/src/loader.rs | 2 +- crates/bevy_asset/src/path.rs | 42 +++++++++++++-------- crates/bevy_asset/src/processor/mod.rs | 16 ++++---- crates/bevy_asset/src/server/mod.rs | 4 +- crates/bevy_utils/src/cow_arc.rs | 26 +++++++++---- 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/crates/bevy_asset/src/io/processor_gated.rs b/crates/bevy_asset/src/io/processor_gated.rs index 26b20e8d31e95..ca37b47ea8029 100644 --- a/crates/bevy_asset/src/io/processor_gated.rs +++ b/crates/bevy_asset/src/io/processor_gated.rs @@ -36,7 +36,7 @@ impl ProcessorGatedReader { ) -> Result, AssetReaderError> { let infos = self.processor_data.asset_infos.read().await; let info = infos - .get(&AssetPath::new(path.to_path_buf())) + .get(&AssetPath::from_path(path.to_path_buf())) .ok_or_else(|| AssetReaderError::NotFound(path.to_owned()))?; Ok(info.file_transaction_lock.read_arc().await) } diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 71fb8dfaf5629..6652f7243bd46 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -436,7 +436,7 @@ impl<'a> LoadContext<'a> { let mut bytes = Vec::new(); reader.read_to_end(&mut bytes).await?; self.loader_dependencies - .insert(AssetPath::new(path.to_owned()), hash); + .insert(AssetPath::from_path(path.to_owned()), hash); Ok(bytes) } diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 514bddafd1c01..4cbf75948f973 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -58,9 +58,23 @@ impl<'a> Display for AssetPath<'a> { } impl<'a> AssetPath<'a> { - /// Creates a new asset path using borrowed information. + /// Creates a new [`AssetPath`] from a string in the asset path format: + /// * An asset at the root: `"scene.gltf"` + /// * An asset nested in some folders: `"some/path/scene.gltf"` + /// * An asset with a "label": `"some/path/scene.gltf#Mesh0"` + pub fn new(asset_path: &'a str) -> AssetPath<'a> { + let mut parts = asset_path.splitn(2, '#'); + let path = Path::new(parts.next().expect("Path must be set.")); + let label = parts.next(); + Self { + path: CowArc::Borrowed(path), + label: label.map(|l| CowArc::Borrowed(l)), + } + } + + /// Creates a new [`AssetPath`] from a [`Path`]. #[inline] - pub fn new(path: impl Into>) -> AssetPath<'a> { + pub fn from_path(path: impl Into>) -> AssetPath<'a> { AssetPath { path: path.into(), label: None, @@ -141,34 +155,30 @@ impl<'a> AssetPath<'a> { } } -impl<'a> From<&'a str> for AssetPath<'a> { - fn from(asset_path: &'a str) -> Self { - let mut parts = asset_path.splitn(2, '#'); - let path = Path::new(parts.next().expect("Path must be set.")); - let label = parts.next(); - Self { - path: path.into(), - label: label.map(|l| l.into()), - } +impl From<&'static str> for AssetPath<'static> { + #[inline] + fn from(asset_path: &'static str) -> Self { + AssetPath::new(asset_path) } } + impl<'a> From<&'a String> for AssetPath<'a> { #[inline] fn from(asset_path: &'a String) -> Self { - asset_path.as_str().into() + AssetPath::new(asset_path.as_str()) } } impl From for AssetPath<'static> { #[inline] fn from(asset_path: String) -> Self { - AssetPath::from(asset_path.as_str()).into_owned() + AssetPath::new(asset_path.as_str()).into_owned() } } -impl<'a> From<&'a Path> for AssetPath<'a> { +impl From<&'static Path> for AssetPath<'static> { #[inline] - fn from(path: &'a Path) -> Self { + fn from(path: &'static Path) -> Self { Self { path: path.into(), label: None, @@ -232,7 +242,7 @@ impl<'de> Visitor<'de> for AssetPathVisitor { where E: serde::de::Error, { - Ok(AssetPath::from(v).into_owned()) + Ok(AssetPath::new(v).into_owned()) } fn visit_string(self, v: String) -> Result diff --git a/crates/bevy_asset/src/processor/mod.rs b/crates/bevy_asset/src/processor/mod.rs index a537063598377..2e67be23deca5 100644 --- a/crates/bevy_asset/src/processor/mod.rs +++ b/crates/bevy_asset/src/processor/mod.rs @@ -364,7 +364,7 @@ impl AssetProcessor { /// asset have finished, thanks to the `file_transaction_lock`. async fn handle_removed_asset(&self, path: PathBuf) { debug!("Removing processed {:?} because source was removed", path); - let asset_path = AssetPath::new(path); + let asset_path = AssetPath::from_path(path); let mut infos = self.data.asset_infos.write().await; if let Some(info) = infos.get(&asset_path) { // we must wait for uncontested write access to the asset source to ensure existing readers / writers @@ -380,7 +380,7 @@ impl AssetProcessor { /// This will cause direct path dependencies to break. async fn handle_renamed_asset(&self, old: PathBuf, new: PathBuf) { let mut infos = self.data.asset_infos.write().await; - let old_asset_path = AssetPath::new(old); + let old_asset_path = AssetPath::from_path(old); if let Some(info) = infos.get(&old_asset_path) { // we must wait for uncontested write access to the asset source to ensure existing readers / writers // can finish their operations @@ -392,7 +392,7 @@ impl AssetProcessor { .await .unwrap(); } - let new_asset_path = AssetPath::new(new); + let new_asset_path = AssetPath::from_path(new); infos.rename(&old_asset_path, &new_asset_path).await; } @@ -531,11 +531,11 @@ impl AssetProcessor { .map_err(InitializeError::FailedToReadDestinationPaths)?; for path in &source_paths { - asset_infos.get_or_insert(AssetPath::new(path.clone())); + asset_infos.get_or_insert(AssetPath::from_path(path.clone())); } for path in &destination_paths { - let asset_path = AssetPath::new(path.clone()); + let asset_path = AssetPath::from_path(path.clone()); let mut dependencies = Vec::new(); if let Some(info) = asset_infos.get_mut(&asset_path) { match self.destination_reader().read_meta_bytes(path).await { @@ -627,7 +627,7 @@ impl AssetProcessor { async fn process_asset(&self, path: &Path) { let result = self.process_asset_internal(path).await; let mut infos = self.data.asset_infos.write().await; - let asset_path = AssetPath::new(path.to_owned()); + let asset_path = AssetPath::from_path(path.to_owned()); infos.finish_processing(asset_path, result).await; } @@ -635,7 +635,7 @@ impl AssetProcessor { if path.extension().is_none() { return Err(ProcessError::ExtensionRequired); } - let asset_path = AssetPath::new(path.to_path_buf()); + let asset_path = AssetPath::from_path(path.to_path_buf()); // TODO: check if already processing to protect against duplicate hot-reload events debug!("Processing {:?}", path); let server = &self.server; @@ -912,7 +912,7 @@ impl AssetProcessorData { self.wait_until_initialized().await; let mut receiver = { let infos = self.asset_infos.write().await; - let info = infos.get(&AssetPath::new(path.to_path_buf())); + let info = infos.get(&AssetPath::from_path(path.to_path_buf())); match info { Some(info) => match info.status { Some(result) => return result, diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index eac1000d20639..b4e8470b13c2d 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -471,7 +471,7 @@ impl AssetServer { load_folder(&child_path, server, handles).await?; } else { let path = child_path.to_str().expect("Path should be a valid string."); - match server.load_untyped_async(path).await { + match server.load_untyped_async(AssetPath::new(path)).await { Ok(handle) => handles.push(handle), // skip assets that cannot be loaded Err( @@ -752,7 +752,7 @@ pub fn handle_internal_asset_events(world: &mut World) { // TODO: if the asset was processed and the processed file was changed, the first modified event // should be skipped? AssetSourceEvent::ModifiedAsset(path) | AssetSourceEvent::ModifiedMeta(path) => { - let path = AssetPath::new(path); + let path = AssetPath::from_path(path); queue_ancestors(&path, &infos, &mut paths_to_reload); paths_to_reload.insert(path); } diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index a555a4e53acad..1de1a015737e4 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -8,9 +8,11 @@ use std::{ /// Much like a [`Cow`](std::borrow::Cow), but owned values are Arc-ed to make clones cheap. This should be used for values that /// are cloned for use across threads and change rarely (if ever). -pub enum CowArc<'a, T: ?Sized> { +pub enum CowArc<'a, T: ?Sized + 'static> { /// A borrowed value Borrowed(&'a T), + /// A static value reference + Static(&'static T), /// An owned [`Arc`]-ed value Owned(Arc), } @@ -22,6 +24,7 @@ impl<'a, T: ?Sized> Deref for CowArc<'a, T> { fn deref(&self) -> &Self::Target { match self { CowArc::Borrowed(v) => v, + CowArc::Static(v) => v, CowArc::Owned(v) => v, } } @@ -37,6 +40,7 @@ where pub fn into_owned(self) -> CowArc<'static, T> { match self { CowArc::Borrowed(value) => CowArc::Owned(value.into()), + CowArc::Static(value) => CowArc::Static(value), CowArc::Owned(value) => CowArc::Owned(value), } } @@ -46,6 +50,7 @@ impl<'a, T: ?Sized> Clone for CowArc<'a, T> { fn clone(&self) -> Self { match self { Self::Borrowed(value) => Self::Borrowed(value), + Self::Static(value) => Self::Static(value), Self::Owned(value) => Self::Owned(value.clone()), } } @@ -96,13 +101,20 @@ impl From for CowArc<'static, Path> { } } -impl<'a> From<&'a str> for CowArc<'a, Path> { +impl From<&'static str> for CowArc<'static, Path> { #[inline] - fn from(value: &'a str) -> Self { - CowArc::Borrowed(Path::new(value)) + fn from(value: &'static str) -> Self { + CowArc::Static(Path::new(value)) } } +// impl<'a> From<&'a str> for CowArc<'a, Path> { +// #[inline] +// fn from(value: &'a str) -> Self { +// CowArc::Borrowed(Path::new(value)) +// } +// } + impl From for CowArc<'static, str> { #[inline] fn from(value: String) -> Self { @@ -117,9 +129,9 @@ impl<'a> From<&'a String> for CowArc<'a, str> { } } -impl<'a, T: ?Sized> From<&'a T> for CowArc<'a, T> { +impl From<&'static T> for CowArc<'static, T> { #[inline] - fn from(value: &'a T) -> Self { - CowArc::Borrowed(value) + fn from(value: &'static T) -> Self { + CowArc::Static(value) } } From 8645800ba384ebde62dcb2298f5f003ee3f877d2 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 12:11:25 -0700 Subject: [PATCH 08/12] Remove commented code --- crates/bevy_utils/src/cow_arc.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index 1de1a015737e4..016c27919a43e 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -108,13 +108,6 @@ impl From<&'static str> for CowArc<'static, Path> { } } -// impl<'a> From<&'a str> for CowArc<'a, Path> { -// #[inline] -// fn from(value: &'a str) -> Self { -// CowArc::Borrowed(Path::new(value)) -// } -// } - impl From for CowArc<'static, str> { #[inline] fn from(value: String) -> Self { From b05457f7da50153b3d99931d1bd85e8259356b43 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 14:22:19 -0700 Subject: [PATCH 09/12] Inline more things --- crates/bevy_utils/src/cow_arc.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index 016c27919a43e..c4d3dfb80de2b 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -47,6 +47,7 @@ where } impl<'a, T: ?Sized> Clone for CowArc<'a, T> { + #[inline] fn clone(&self) -> Self { match self { Self::Borrowed(value) => Self::Borrowed(value), @@ -57,6 +58,7 @@ impl<'a, T: ?Sized> Clone for CowArc<'a, T> { } impl<'a, T: PartialEq + ?Sized> PartialEq for CowArc<'a, T> { + #[inline] fn eq(&self, other: &Self) -> bool { self.deref().eq(other.deref()) } @@ -65,30 +67,35 @@ impl<'a, T: PartialEq + ?Sized> PartialEq for CowArc<'a, T> { impl<'a, T: PartialEq + ?Sized> Eq for CowArc<'a, T> {} impl<'a, T: Hash + ?Sized> Hash for CowArc<'a, T> { + #[inline] fn hash(&self, state: &mut H) { self.deref().hash(state); } } impl<'a, T: Debug + ?Sized> Debug for CowArc<'a, T> { + #[inline] fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Debug::fmt(self.deref(), f) } } impl<'a, T: Display + ?Sized> Display for CowArc<'a, T> { + #[inline] fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Display::fmt(self.deref(), f) } } impl<'a, T: PartialOrd + ?Sized> PartialOrd for CowArc<'a, T> { + #[inline] fn partial_cmp(&self, other: &Self) -> Option { self.deref().partial_cmp(other.deref()) } } impl<'a, T: Ord + ?Sized> Ord for CowArc<'a, T> { + #[inline] fn cmp(&self, other: &Self) -> std::cmp::Ordering { self.deref().cmp(other.deref()) } From 51bfb445367359de96be5c7453ecea176a36c8c3 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 14:23:48 -0700 Subject: [PATCH 10/12] Clippy --- crates/bevy_utils/src/cow_arc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index c4d3dfb80de2b..5dcef887badff 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -23,8 +23,7 @@ impl<'a, T: ?Sized> Deref for CowArc<'a, T> { #[inline] fn deref(&self) -> &Self::Target { match self { - CowArc::Borrowed(v) => v, - CowArc::Static(v) => v, + CowArc::Borrowed(v) | CowArc::Static(v) => v, CowArc::Owned(v) => v, } } From d82e8296e226deea262b67c8d30a04c971a2b895 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 14:29:20 -0700 Subject: [PATCH 11/12] Improve CowArc::Static docs --- crates/bevy_utils/src/cow_arc.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/cow_arc.rs b/crates/bevy_utils/src/cow_arc.rs index 5dcef887badff..5e1259a82b82c 100644 --- a/crates/bevy_utils/src/cow_arc.rs +++ b/crates/bevy_utils/src/cow_arc.rs @@ -8,10 +8,18 @@ use std::{ /// Much like a [`Cow`](std::borrow::Cow), but owned values are Arc-ed to make clones cheap. This should be used for values that /// are cloned for use across threads and change rarely (if ever). +/// +/// This also makes an opinionated tradeoff by adding a [`CowArc::Static`] and implementing [`From<&'static T>`] instead of +/// [`From<'a T>`]. This preserves the static context and prevents conversion to [`CowArc::Owned`] in cases where a reference +/// is known to be static. This is an optimization that prevents allocations and atomic ref-counting. +/// +/// This means that static references should prefer [`From::from`] or [`CowArc::Static`] and non-static references must +/// use [`CowArc::Borrowed`]. pub enum CowArc<'a, T: ?Sized + 'static> { /// A borrowed value Borrowed(&'a T), - /// A static value reference + /// A static value reference. This exists to avoid conversion to [`CowArc::Owned`] in cases where a reference is + /// known to be static. This is an optimization that prevents allocations and atomic ref-counting. Static(&'static T), /// An owned [`Arc`]-ed value Owned(Arc), From 03b3516021725e76ce0983f90b784375a0258de2 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 9 Sep 2023 14:44:18 -0700 Subject: [PATCH 12/12] Actually use CowArc::Static! --- crates/bevy_asset/src/path.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 4cbf75948f973..9c0111006e73a 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -34,6 +34,12 @@ use std::{ /// // This loads the `PlayerMesh` labeled asset from the `my_scene.scn` base asset. /// let mesh: Handle = asset_server.load("my_scene.scn#PlayerMesh"); /// ``` +/// +/// [`AssetPath`] implements [`From`] for `&'static str`, `&'static Path`, and `&'a String`, +/// which allows us to optimize the static cases. +/// This means that the common case of `asset_server.load("my_scene.scn")` when it creates and +/// clones internal owned [`AssetPaths`](AssetPath). +/// This also means that you should use [`AssetPath::new`] in cases where `&str` is the explicit type. #[derive(Eq, PartialEq, Hash, Clone, Reflect)] #[reflect(Debug, PartialEq, Hash, Serialize, Deserialize)] pub struct AssetPath<'a> { @@ -62,16 +68,24 @@ impl<'a> AssetPath<'a> { /// * An asset at the root: `"scene.gltf"` /// * An asset nested in some folders: `"some/path/scene.gltf"` /// * An asset with a "label": `"some/path/scene.gltf#Mesh0"` + /// + /// Prefer [`From<'static str>`] for static strings, as this will prevent allocations + /// and reference counting for [`AssetPath::into_owned`]. pub fn new(asset_path: &'a str) -> AssetPath<'a> { - let mut parts = asset_path.splitn(2, '#'); - let path = Path::new(parts.next().expect("Path must be set.")); - let label = parts.next(); + let (path, label) = Self::get_parts(asset_path); Self { path: CowArc::Borrowed(path), - label: label.map(|l| CowArc::Borrowed(l)), + label: label.map(CowArc::Borrowed), } } + fn get_parts(asset_path: &str) -> (&Path, Option<&str>) { + let mut parts = asset_path.splitn(2, '#'); + let path = Path::new(parts.next().expect("Path must be set.")); + let label = parts.next(); + (path, label) + } + /// Creates a new [`AssetPath`] from a [`Path`]. #[inline] pub fn from_path(path: impl Into>) -> AssetPath<'a> { @@ -158,7 +172,11 @@ impl<'a> AssetPath<'a> { impl From<&'static str> for AssetPath<'static> { #[inline] fn from(asset_path: &'static str) -> Self { - AssetPath::new(asset_path) + let (path, label) = Self::get_parts(asset_path); + AssetPath { + path: CowArc::Static(path), + label: label.map(CowArc::Static), + } } } @@ -180,7 +198,7 @@ impl From<&'static Path> for AssetPath<'static> { #[inline] fn from(path: &'static Path) -> Self { Self { - path: path.into(), + path: CowArc::Static(path), label: None, } }