Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1657e22

Browse filesBrowse files
makspllmakspll
committed
change approach, simplify access map, stop checking threads
1 parent 158cde2 commit 1657e22
Copy full SHA for 1657e22

File tree

Expand file treeCollapse file tree

1 file changed

+49
-88
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+49
-88
lines changed

‎crates/bevy_mod_scripting_core/src/bindings/access_map.rs

Copy file name to clipboardExpand all lines: crates/bevy_mod_scripting_core/src/bindings/access_map.rs
+49-88Lines changed: 49 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//! A map of access claims used to safely and dynamically access the world.
2-
use std::thread::ThreadId;
32
43
use bevy::{
54
ecs::{component::ComponentId, world::unsafe_world_cell::UnsafeWorldCell},
@@ -16,7 +15,6 @@ use super::{ReflectAllocationId, ReflectBase};
1615
#[derive(Debug, Clone, PartialEq, Eq)]
1716
/// An owner of an access claim and the code location of the claim.
1817
pub struct ClaimOwner {
19-
id: ThreadId,
2018
location: std::panic::Location<'static>,
2119
}
2220

@@ -35,6 +33,7 @@ impl Default for AccessCount {
3533
}
3634
}
3735

36+
#[profiling::all_functions]
3837
impl AccessCount {
3938
fn new() -> Self {
4039
Self {
@@ -71,6 +70,7 @@ pub trait AccessMapKey {
7170
fn from_index(value: u64) -> Self;
7271
}
7372

73+
#[profiling::all_functions]
7474
impl AccessMapKey for u64 {
7575
fn as_index(&self) -> u64 {
7676
*self
@@ -100,6 +100,7 @@ pub struct ReflectAccessId {
100100
pub(crate) id: u64,
101101
}
102102

103+
#[profiling::all_functions]
103104
impl AccessMapKey for ReflectAccessId {
104105
fn as_index(&self) -> u64 {
105106
// project two linear non-negative ranges [0,inf] to a single linear non-negative range, offset by 1 to avoid 0
@@ -134,6 +135,7 @@ impl AccessMapKey for ReflectAccessId {
134135
}
135136
}
136137

138+
#[profiling::all_functions]
137139
impl ReflectAccessId {
138140
/// Creates a new access id for the global world
139141
pub fn for_global() -> Self {
@@ -192,6 +194,7 @@ impl ReflectAccessId {
192194
}
193195
}
194196

197+
#[profiling::all_functions]
195198
impl From<ComponentId> for ReflectAccessId {
196199
fn from(value: ComponentId) -> Self {
197200
Self {
@@ -201,6 +204,7 @@ impl From<ComponentId> for ReflectAccessId {
201204
}
202205
}
203206

207+
#[profiling::all_functions]
204208
impl From<ReflectAllocationId> for ReflectAccessId {
205209
fn from(value: ReflectAllocationId) -> Self {
206210
Self {
@@ -210,12 +214,14 @@ impl From<ReflectAllocationId> for ReflectAccessId {
210214
}
211215
}
212216

217+
#[profiling::all_functions]
213218
impl From<ReflectAccessId> for ComponentId {
214219
fn from(val: ReflectAccessId) -> Self {
215220
ComponentId::new(val.id as usize)
216221
}
217222
}
218223

224+
#[profiling::all_functions]
219225
impl From<ReflectAccessId> for ReflectAllocationId {
220226
fn from(val: ReflectAccessId) -> Self {
221227
ReflectAllocationId::new(val.id)
@@ -315,6 +321,29 @@ struct AccessMapInner {
315321
global_lock: AccessCount,
316322
}
317323

324+
#[profiling::all_functions]
325+
impl AccessMapInner {
326+
#[inline]
327+
fn entry(&self, key: u64) -> Option<&AccessCount> {
328+
self.individual_accesses.get(&key)
329+
}
330+
331+
#[inline]
332+
fn entry_mut(&mut self, key: u64) -> Option<&mut AccessCount> {
333+
self.individual_accesses.get_mut(&key)
334+
}
335+
336+
#[inline]
337+
fn entry_or_default(&mut self, key: u64) -> &mut AccessCount {
338+
self.individual_accesses.entry(key).or_default()
339+
}
340+
341+
#[inline]
342+
fn remove(&mut self, key: u64) {
343+
self.individual_accesses.remove(&key);
344+
}
345+
}
346+
318347
const GLOBAL_KEY: u64 = 0;
319348

320349
#[profiling::all_functions]
@@ -362,10 +391,10 @@ impl DynamicSystemMeta for AccessMap {
362391
return false;
363392
}
364393

365-
let entry = inner.individual_accesses.entry(key).or_default();
394+
let entry = inner.entry_or_default(key);
395+
366396
if entry.can_read() {
367397
entry.read_by.push(ClaimOwner {
368-
id: std::thread::current().id(),
369398
location: *std::panic::Location::caller(),
370399
});
371400
true
@@ -388,10 +417,10 @@ impl DynamicSystemMeta for AccessMap {
388417
return false;
389418
}
390419

391-
let entry = inner.individual_accesses.entry(key).or_default();
420+
let entry = inner.entry_or_default(key);
421+
392422
if entry.can_write() {
393423
entry.read_by.push(ClaimOwner {
394-
id: std::thread::current().id(),
395424
location: *std::panic::Location::caller(),
396425
});
397426
entry.written = true;
@@ -409,7 +438,6 @@ impl DynamicSystemMeta for AccessMap {
409438
return false;
410439
}
411440
inner.global_lock.read_by.push(ClaimOwner {
412-
id: std::thread::current().id(),
413441
location: *std::panic::Location::caller(),
414442
});
415443
inner.global_lock.written = true;
@@ -420,39 +448,27 @@ impl DynamicSystemMeta for AccessMap {
420448
let mut inner = self.0.lock();
421449
let key = key.as_index();
422450

423-
if let Some(entry) = inner.individual_accesses.get_mut(&key) {
451+
if let Some(entry) = inner.entry_mut(key) {
424452
entry.written = false;
425-
if let Some(claim) = entry.read_by.pop() {
426-
assert!(
427-
claim.id == std::thread::current().id(),
428-
"Access released from wrong thread, claimed at {}",
429-
claim.location.display_location()
430-
);
431-
}
453+
entry.read_by.pop();
432454
if entry.readers() == 0 {
433-
inner.individual_accesses.remove(&key);
455+
inner.remove(key);
434456
}
435457
}
436458
}
437459

438460
fn release_global_access(&self) {
439461
let mut inner = self.0.lock();
462+
inner.global_lock.read_by.pop();
440463
inner.global_lock.written = false;
441-
if let Some(claim) = inner.global_lock.read_by.pop() {
442-
assert!(
443-
claim.id == std::thread::current().id(),
444-
"Global access released from wrong thread, claimed at {}",
445-
claim.location.display_location()
446-
);
447-
}
448464
}
449465

450466
fn list_accesses<K: AccessMapKey>(&self) -> Vec<(K, AccessCount)> {
451467
let inner = self.0.lock();
452468
inner
453469
.individual_accesses
454470
.iter()
455-
.map(|(&key, count)| (K::from_index(key), count.clone()))
471+
.map(|(key, a)| (K::from_index(*key), a.clone()))
456472
.collect()
457473
}
458474

@@ -486,8 +502,7 @@ impl DynamicSystemMeta for AccessMap {
486502
})
487503
} else {
488504
inner
489-
.individual_accesses
490-
.get(&key.as_index())
505+
.entry(key.as_index())
491506
.and_then(|access| access.as_location())
492507
}
493508
}
@@ -496,8 +511,9 @@ impl DynamicSystemMeta for AccessMap {
496511
let inner = self.0.lock();
497512
inner
498513
.individual_accesses
499-
.values()
500-
.find_map(|access| access.as_location())
514+
.iter()
515+
.next()
516+
.and_then(|(_, access)| access.as_location())
501517
}
502518
}
503519

@@ -507,6 +523,7 @@ pub struct SubsetAccessMap {
507523
subset: Box<dyn Fn(u64) -> bool + Send + Sync + 'static>,
508524
}
509525

526+
#[profiling::all_functions]
510527
impl SubsetAccessMap {
511528
/// Creates a new subset access map with the provided subset of ID's as well as a exception function.
512529
pub fn new(
@@ -528,6 +545,7 @@ impl SubsetAccessMap {
528545
}
529546
}
530547

548+
#[profiling::all_functions]
531549
impl DynamicSystemMeta for SubsetAccessMap {
532550
fn with_scope<O, F: FnOnce() -> O>(&self, f: F) -> O {
533551
self.inner.with_scope(f)
@@ -601,6 +619,7 @@ pub enum AnyAccessMap {
601619
SubsetAccessMap(SubsetAccessMap),
602620
}
603621

622+
#[profiling::all_functions]
604623
impl DynamicSystemMeta for AnyAccessMap {
605624
fn with_scope<O, F: FnOnce() -> O>(&self, f: F) -> O {
606625
match self {
@@ -700,12 +719,14 @@ pub trait DisplayCodeLocation {
700719
fn display_location(self) -> String;
701720
}
702721

722+
#[profiling::all_functions]
703723
impl DisplayCodeLocation for std::panic::Location<'_> {
704724
fn display_location(self) -> String {
705725
format!("\"{}:{}\"", self.file(), self.line())
706726
}
707727
}
708728

729+
#[profiling::all_functions]
709730
impl DisplayCodeLocation for Option<std::panic::Location<'_>> {
710731
fn display_location(self) -> String {
711732
self.map(|l| l.display_location())
@@ -926,66 +947,6 @@ mod test {
926947
assert!(!subset_access_map.claim_global_access());
927948
}
928949

929-
#[test]
930-
#[should_panic]
931-
fn access_map_releasing_read_access_from_wrong_thread_panics() {
932-
let access_map = AccessMap::default();
933-
934-
access_map.claim_read_access(1);
935-
std::thread::spawn(move || {
936-
access_map.release_access(1);
937-
})
938-
.join()
939-
.unwrap();
940-
}
941-
942-
#[test]
943-
#[should_panic]
944-
fn subset_map_releasing_read_access_from_wrong_thread_panics() {
945-
let access_map = AccessMap::default();
946-
let subset_access_map = SubsetAccessMap {
947-
inner: access_map,
948-
subset: Box::new(|id| id == 1),
949-
};
950-
951-
subset_access_map.claim_read_access(1);
952-
std::thread::spawn(move || {
953-
subset_access_map.release_access(1);
954-
})
955-
.join()
956-
.unwrap();
957-
}
958-
959-
#[test]
960-
#[should_panic]
961-
fn access_map_releasing_write_access_from_wrong_thread_panics() {
962-
let access_map = AccessMap::default();
963-
964-
access_map.claim_write_access(1);
965-
std::thread::spawn(move || {
966-
access_map.release_access(1);
967-
})
968-
.join()
969-
.unwrap();
970-
}
971-
972-
#[test]
973-
#[should_panic]
974-
fn subset_map_releasing_write_access_from_wrong_thread_panics() {
975-
let access_map = AccessMap::default();
976-
let subset_access_map = SubsetAccessMap {
977-
inner: access_map,
978-
subset: Box::new(|id| id == 1),
979-
};
980-
981-
subset_access_map.claim_write_access(1);
982-
std::thread::spawn(move || {
983-
subset_access_map.release_access(1);
984-
})
985-
.join()
986-
.unwrap();
987-
}
988-
989950
#[test]
990951
fn as_and_from_index_for_access_id_non_overlapping() {
991952
let global = ReflectAccessId::for_global();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.