Skip to content

Commit e898794

Browse files
authored
Merge pull request #224 from Imberflur/replace-trustcell-with-atomicrefcell
2 parents 493d504 + b57f065 commit e898794

File tree

8 files changed

+78
-730
lines changed

8 files changed

+78
-730
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ rust-version = "1.59.0"
1919
[dependencies]
2020
ahash = "0.7.6"
2121
arrayvec = "0.7.2"
22+
atomic_refcell = "0.1.10" # part of public API
2223
rayon = { version = "1.5.0", optional = true }
2324
shred-derive = { path = "shred-derive", version = "0.6.3", optional = true }
2425
smallvec = "1.6.1"

benches/bench.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,11 +241,14 @@ fn bench_fetching(b: &mut Bencher) {
241241

242242
#[bench]
243243
fn bench_indirection_refs(b: &mut Bencher) {
244-
use shred::cell::{Ref, TrustCell};
244+
use shred::cell::{AtomicRef, AtomicRefCell};
245245
use std::ops::Deref;
246246

247-
let cell = TrustCell::new(Box::new(10));
248-
let refs: Vec<Ref<'_, Box<usize>>> = std::iter::repeat(cell.borrow()).take(10000).collect();
247+
let cell = AtomicRefCell::new(Box::new(10));
248+
let borrow = cell.borrow();
249+
let refs: Vec<AtomicRef<'_, Box<usize>>> = std::iter::repeat_with(|| AtomicRef::clone(&borrow))
250+
.take(10000)
251+
.collect();
249252

250253
b.iter(|| {
251254
let sum: usize = refs.iter().map(|v| v.deref().deref()).sum();
@@ -255,13 +258,15 @@ fn bench_indirection_refs(b: &mut Bencher) {
255258

256259
#[bench]
257260
fn bench_direct_refs(b: &mut Bencher) {
258-
use shred::cell::{Ref, TrustCell};
261+
use shred::cell::{AtomicRef, AtomicRefCell};
259262
use std::ops::Deref;
260263

261-
let cell = TrustCell::new(Box::new(10));
262-
let refs: Vec<Ref<'_, usize>> = std::iter::repeat(cell.borrow().map(Box::as_ref))
263-
.take(10000)
264-
.collect();
264+
let cell = AtomicRefCell::new(Box::new(10));
265+
let mapped_borrow = AtomicRef::map(cell.borrow(), Box::as_ref);
266+
let refs: Vec<AtomicRef<'_, usize>> =
267+
std::iter::repeat_with(|| AtomicRef::clone(&mapped_borrow))
268+
.take(10000)
269+
.collect();
265270

266271
b.iter(|| {
267272
let sum: usize = refs.iter().map(|v| v.deref()).sum();

examples/dyn_sys_data.rs

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ extern crate shred;
1111
use ahash::AHashMap as HashMap;
1212

1313
use shred::{
14-
cell::{Ref, RefMut},
14+
cell::{AtomicRef, AtomicRefMut},
1515
Accessor, AccessorCow, CastFrom, DispatcherBuilder, DynamicSystemData, MetaTable, Read,
1616
Resource, ResourceId, System, SystemData, World,
1717
};
@@ -54,32 +54,20 @@ impl<'a> System<'a> for DynamicSystem {
5454
let reads: Vec<&dyn Reflection> = data
5555
.reads
5656
.iter()
57-
.map(|resource| {
58-
// explicitly use the type because we're dealing with `&Resource` which is
59-
// implemented by a lot of types; we don't want to accidentally
60-
// get a `&Box<Resource>` and cast it to a `&Resource`.
61-
let res = Box::as_ref(resource);
62-
63-
meta.get(res).expect("Not registered in meta table")
64-
})
57+
.map(|resource| meta.get(&**resource).expect("Not registered in meta table"))
6558
.collect();
6659

6760
let writes: Vec<&mut dyn Reflection> = data
6861
.writes
6962
.iter_mut()
7063
.map(|resource| {
71-
// explicitly use the type because we're dealing with `&mut Resource` which is
72-
// implemented by a lot of types; we don't want to accidentally get a
73-
// `&mut Box<Resource>` and cast it to a `&mut Resource`.
74-
let res = Box::as_mut(resource);
75-
7664
// For some reason this needs a type ascription, otherwise Rust will think it's
77-
// a `&mut (Reflection + '_)` (as opposed to `&mut (Reflection + 'static)`.
78-
let res: &mut dyn Reflection = meta.get_mut(res).expect(
65+
// a `&mut (Reflaction + '_)` (as opposed to `&mut (Reflection + 'static)`. (Note this
66+
// isn't needed in newer rust version but fails on the MSRV of 1.59.0).
67+
let res: &mut dyn Reflection = meta.get_mut(&mut **resource).expect(
7968
"Not registered in meta \
8069
table",
8170
);
82-
8371
res
8472
})
8573
.collect();
@@ -150,37 +138,47 @@ struct ScriptInput<'a> {
150138

151139
struct ScriptSystemData<'a> {
152140
meta_table: Read<'a, ReflectionTable>,
153-
reads: Vec<Ref<'a, Box<dyn Resource + 'static>>>,
154-
writes: Vec<RefMut<'a, Box<dyn Resource + 'static>>>,
141+
reads: Vec<AtomicRef<'a, dyn Resource + 'static>>,
142+
writes: Vec<AtomicRefMut<'a, dyn Resource + 'static>>,
155143
}
156144

157145
impl<'a> DynamicSystemData<'a> for ScriptSystemData<'a> {
158146
type Accessor = Dependencies;
159147

160148
fn setup(_accessor: &Dependencies, _res: &mut World) {}
161149

162-
fn fetch(access: &Dependencies, res: &'a World) -> Self {
150+
fn fetch(access: &Dependencies, world: &'a World) -> Self {
163151
let reads = access
164152
.reads
165153
.iter()
166154
.map(|id| {
167-
res.try_fetch_internal(id.clone())
168-
.expect("bug: the requested resource does not exist")
169-
.borrow()
155+
let id = id.clone();
156+
// SAFETY: We don't expose mutable reference to the Box or swap it out.
157+
let res = unsafe { world.try_fetch_internal(id) };
158+
AtomicRef::map(
159+
res.expect("bug: the requested resource does not exist")
160+
.borrow(),
161+
Box::as_ref,
162+
)
170163
})
171164
.collect();
172165
let writes = access
173166
.writes
174167
.iter()
175168
.map(|id| {
176-
res.try_fetch_internal(id.clone())
177-
.expect("bug: the requested resource does not exist")
178-
.borrow_mut()
169+
let id = id.clone();
170+
// SAFETY: We don't expose mutable reference to the Box or swap it out.
171+
let res = unsafe { world.try_fetch_internal(id) };
172+
AtomicRefMut::map(
173+
res.expect("bug: the requested resource does not exist")
174+
.borrow_mut(),
175+
Box::as_mut,
176+
)
179177
})
180178
.collect();
181179

182180
ScriptSystemData {
183-
meta_table: SystemData::fetch(res),
181+
meta_table: SystemData::fetch(world),
184182
reads,
185183
writes,
186184
}

0 commit comments

Comments
 (0)