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.

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

Replaced parking_lot with std::sync #9545

Merged
merged 4 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 0 additions & 1 deletion crates/bevy_asset/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ thiserror = "1.0"
downcast-rs = "1.2.0"
fastrand = "1.7.0"
notify = { version = "6.0.0", optional = true }
parking_lot = "0.12.1"
async-channel = "1.4.2"

[target.'cfg(target_os = "android")'.dependencies]
Expand Down
133 changes: 110 additions & 23 deletions crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ use bevy_log::warn;
use bevy_tasks::IoTaskPool;
use bevy_utils::{Entry, HashMap, Uuid};
use crossbeam_channel::TryRecvError;
use parking_lot::{Mutex, RwLock};
use std::{path::Path, sync::Arc};
use std::{
path::Path,
sync::{Arc, Mutex, RwLock},
};
use thiserror::Error;

/// Errors that occur while loading assets with an [`AssetServer`].
Expand Down Expand Up @@ -157,6 +159,7 @@ impl AssetServer {
.server
.asset_lifecycles
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.insert(T::TYPE_UUID, Box::<AssetLifecycleChannel<T>>::default())
.is_some()
{
Expand All @@ -171,13 +174,18 @@ impl AssetServer {
/// Assets loaded with matching extensions will be blocked until the
/// real loader is added.
pub fn preregister_loader(&self, extensions: &[&str]) {
let mut loaders = self.server.loaders.write();
let mut loaders = self
.server
.loaders
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let loader_index = loaders.len();
for extension in extensions {
if self
.server
.extension_to_loader_index
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.insert(extension.to_string(), loader_index)
.is_some()
{
Expand All @@ -196,10 +204,18 @@ impl AssetServer {
where
T: AssetLoader,
{
let mut loaders = self.server.loaders.write();
let mut loaders = self
.server
.loaders
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let next_loader_index = loaders.len();
let mut maybe_existing_loader_index = None;
let mut loader_map = self.server.extension_to_loader_index.write();
let mut loader_map = self
.server
.extension_to_loader_index
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let mut maybe_sender = None;

for extension in loader.extensions() {
Expand Down Expand Up @@ -258,11 +274,21 @@ impl AssetServer {
fn get_asset_loader(&self, extension: &str) -> Result<MaybeAssetLoader, AssetServerError> {
let index = {
// scope map to drop lock as soon as possible
let map = self.server.extension_to_loader_index.read();
let map = self
.server
.extension_to_loader_index
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
map.get(extension).copied()
};
index
.map(|index| self.server.loaders.read()[index].clone())
.map(|index| {
self.server
.loaders
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)[index]
.clone()
})
.ok_or_else(|| AssetServerError::MissingAssetLoader {
extensions: vec![extension.to_string()],
})
Expand Down Expand Up @@ -306,6 +332,7 @@ impl AssetServer {
self.server
.handle_to_path
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.get(&handle.into())
.cloned()
}
Expand All @@ -314,7 +341,11 @@ impl AssetServer {
pub fn get_load_state<H: Into<HandleId>>(&self, handle: H) -> LoadState {
match handle.into() {
HandleId::AssetPathId(id) => {
let asset_sources = self.server.asset_sources.read();
let asset_sources = self
.server
.asset_sources
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
asset_sources
.get(&id.source_path_id())
.map_or(LoadState::NotLoaded, |info| info.load_state)
Expand Down Expand Up @@ -385,7 +416,11 @@ impl AssetServer {
// load metadata and update source info. this is done in a scope to ensure we release the
// locks before loading
let version = {
let mut asset_sources = self.server.asset_sources.write();
let mut asset_sources = self
.server
.asset_sources
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let source_info = match asset_sources.entry(asset_path_id.source_path_id()) {
Entry::Occupied(entry) => entry.into_mut(),
Entry::Vacant(entry) => entry.insert(SourceInfo {
Expand Down Expand Up @@ -416,7 +451,11 @@ impl AssetServer {
};

let set_asset_failed = || {
let mut asset_sources = self.server.asset_sources.write();
let mut asset_sources = self
.server
.asset_sources
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let source_info = asset_sources
.get_mut(&asset_path_id.source_path_id())
.expect("`AssetSource` should exist at this point.");
Expand Down Expand Up @@ -469,7 +508,11 @@ impl AssetServer {

// if version has changed since we loaded and grabbed a lock, return. there is a newer
// version being loaded
let mut asset_sources = self.server.asset_sources.write();
let mut asset_sources = self
.server
.asset_sources
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let source_info = asset_sources
.get_mut(&asset_path_id.source_path_id())
.expect("`AssetSource` should exist at this point.");
Expand Down Expand Up @@ -540,6 +583,7 @@ impl AssetServer {
self.server
.handle_to_path
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.entry(handle_id)
.or_insert_with(|| asset_path.to_owned());

Expand Down Expand Up @@ -585,12 +629,30 @@ impl AssetServer {

/// Frees unused assets, unloading them from memory.
pub fn free_unused_assets(&self) {
let mut potential_frees = self.server.asset_ref_counter.mark_unused_assets.lock();
let mut potential_frees = self
.server
.asset_ref_counter
.mark_unused_assets
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);

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();
let ref_counts = self
.server
.asset_ref_counter
.ref_counts
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let asset_sources = self
.server
.asset_sources
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let asset_lifecycles = self
.server
.asset_lifecycles
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
for potential_free in potential_frees.drain(..) {
if let Some(&0) = ref_counts.get(&potential_free) {
let type_uuid = match potential_free {
Expand All @@ -613,7 +675,12 @@ impl AssetServer {
/// Iterates through asset references and marks assets with no active handles as unused.
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 ref_counts = self
.server
.asset_ref_counter
.ref_counts
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let mut potential_frees = None;
loop {
let ref_change = match receiver.try_recv() {
Expand All @@ -629,7 +696,11 @@ impl AssetServer {
if *entry == 0 {
potential_frees
.get_or_insert_with(|| {
self.server.asset_ref_counter.mark_unused_assets.lock()
self.server
.asset_ref_counter
.mark_unused_assets
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
})
.push(handle_id);
}
Expand All @@ -639,7 +710,11 @@ impl AssetServer {
}

fn create_assets_in_load_context(&self, load_context: &mut LoadContext) {
let asset_lifecycles = self.server.asset_lifecycles.read();
let asset_lifecycles = self
.server
.asset_lifecycles
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
for (label, asset) in &mut load_context.labeled_assets {
let asset_value = asset
.value
Expand All @@ -664,7 +739,11 @@ impl AssetServer {
// Note: this takes a `ResMut<Assets<T>>` to ensure change detection does not get
// triggered unless the `Assets` collection is actually updated.
pub(crate) fn update_asset_storage<T: Asset>(&self, mut assets: ResMut<Assets<T>>) {
let asset_lifecycles = self.server.asset_lifecycles.read();
let asset_lifecycles = self
.server
.asset_lifecycles
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let asset_lifecycle = asset_lifecycles.get(&T::TYPE_UUID).unwrap();
let mut asset_sources_guard = None;
let channel = asset_lifecycle
Expand All @@ -676,8 +755,12 @@ impl AssetServer {
Ok(AssetLifecycleEvent::Create(result)) => {
// update SourceInfo if this asset was loaded from an AssetPath
if let HandleId::AssetPathId(id) = result.id {
let asset_sources = asset_sources_guard
.get_or_insert_with(|| self.server.asset_sources.write());
let asset_sources = asset_sources_guard.get_or_insert_with(|| {
self.server
.asset_sources
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
});
if let Some(source_info) = asset_sources.get_mut(&id.source_path_id()) {
if source_info.version == result.version {
source_info.committed_assets.insert(id.label_id());
Expand All @@ -692,8 +775,12 @@ impl AssetServer {
}
Ok(AssetLifecycleEvent::Free(handle_id)) => {
if let HandleId::AssetPathId(id) = handle_id {
let asset_sources = asset_sources_guard
.get_or_insert_with(|| self.server.asset_sources.write());
let asset_sources = asset_sources_guard.get_or_insert_with(|| {
self.server
.asset_sources
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
});
if let Some(source_info) = asset_sources.get_mut(&id.source_path_id()) {
source_info.committed_assets.remove(&id.label_id());
source_info.load_state = LoadState::Unloaded;
Expand Down
20 changes: 14 additions & 6 deletions crates/bevy_asset/src/io/file_asset_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ use bevy_utils::{default, HashMap, Instant};
use crossbeam_channel::TryRecvError;
use fs::File;
#[cfg(feature = "filesystem_watcher")]
use parking_lot::RwLock;
#[cfg(feature = "filesystem_watcher")]
use std::sync::Arc;
use std::sync::{Arc, RwLock};
use std::{
convert::TryFrom,
env, fs,
Expand Down Expand Up @@ -132,7 +130,10 @@ impl AssetIo for FileAssetIo {
{
let to_reload = to_reload.unwrap_or_else(|| to_watch.to_owned());
let to_watch = self.root_path.join(to_watch);
let mut watcher = self.filesystem_watcher.write();
let mut watcher = self
.filesystem_watcher
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
if let Some(ref mut watcher) = *watcher {
watcher
.watch(&to_watch, to_reload)
Expand All @@ -146,7 +147,11 @@ impl AssetIo for FileAssetIo {
fn watch_for_changes(&self, configuration: &ChangeWatcher) -> Result<(), AssetIoError> {
#[cfg(feature = "filesystem_watcher")]
{
*self.filesystem_watcher.write() = Some(FilesystemWatcher::new(configuration));
*self
.filesystem_watcher
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner) =
Some(FilesystemWatcher::new(configuration));
}
#[cfg(not(feature = "filesystem_watcher"))]
bevy_log::warn!("Watching for changes is not supported when the `filesystem_watcher` feature is disabled");
Expand Down Expand Up @@ -184,7 +189,10 @@ pub fn filesystem_watcher_system(
} else {
return;
};
let watcher = asset_io.filesystem_watcher.read();
let watcher = asset_io
.filesystem_watcher
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner);

if let Some(ref watcher) = *watcher {
loop {
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_audio/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ bevy_utils = { path = "../bevy_utils", version = "0.12.0-dev" }
# other
anyhow = "1.0.4"
rodio = { version = "0.17", default-features = false }
parking_lot = "0.12.1"

[target.'cfg(target_os = "android")'.dependencies]
oboe = { version = "0.5", optional = true }
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_reflect/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ bevy_ptr = { path = "../bevy_ptr", version = "0.12.0-dev" }
# other
erased-serde = "0.3"
downcast-rs = "1.2"
parking_lot = "0.12.1"
thiserror = "1.0"
once_cell = "1.11"
serde = "1"
Expand Down
22 changes: 17 additions & 5 deletions crates/bevy_reflect/src/type_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ use crate::{serde::Serializable, Reflect, TypeInfo, Typed};
use bevy_ptr::{Ptr, PtrMut};
use bevy_utils::{HashMap, HashSet};
use downcast_rs::{impl_downcast, Downcast};
use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard};
use serde::Deserialize;
use std::{any::TypeId, fmt::Debug, sync::Arc};
use std::{
any::TypeId,
fmt::Debug,
sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard},
};

/// A registry of [reflected] types.
///
Expand Down Expand Up @@ -35,7 +38,12 @@ pub struct TypeRegistryArc {

impl Debug for TypeRegistryArc {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.internal.read().full_name_to_id.keys().fmt(f)
self.internal
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.full_name_to_id
.keys()
.fmt(f)
}
}

Expand Down Expand Up @@ -267,12 +275,16 @@ impl TypeRegistry {
impl TypeRegistryArc {
/// Takes a read lock on the underlying [`TypeRegistry`].
pub fn read(&self) -> RwLockReadGuard<'_, TypeRegistry> {
self.internal.read()
self.internal
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}

/// Takes a write lock on the underlying [`TypeRegistry`].
pub fn write(&self) -> RwLockWriteGuard<'_, TypeRegistry> {
self.internal.write()
self.internal
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner)
}
}

Expand Down
Loading