Skip to content

Commit 2ddbf3f

Browse files
committed
types: use GhostCell in union_bound rather than mutexes
This eliminates all the locking code from union_bound.rs, turning any lifetime mistake into compiler errors rather than deadlocks :). It also lets us simplify a fair bit of code, removing all the drops, all the lock().unwrap()s, and one shallow_clone. There are some borrow()s and borrow_mut()s, but if these compile then they are no-ops, so the reader can basically ignore them. The previous benchmark was: 70,972.00 ns/iter (+/- 662.39) The new one is: 69,947.54 ns/iter (+/- 286.68) which is a ~1.5% improvement, if these numbers are to be believed.
1 parent c749135 commit 2ddbf3f

File tree

3 files changed

+98
-104
lines changed

3 files changed

+98
-104
lines changed

src/types/context.rs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ use ghost_cell::GhostToken;
2222

2323
use crate::dag::{Dag, DagLike};
2424

25-
use super::{Bound, CompleteBound, Error, Final, Incomplete, Type, TypeInner, WithGhostToken};
25+
use super::{
26+
Bound, CompleteBound, Error, Final, Incomplete, Type, TypeInner, UbElement, WithGhostToken,
27+
};
2628

2729
// Copied from ghost_cell source. See
2830
// https://arhan.sh/blog/the-generativity-pattern-in-rust/
@@ -78,7 +80,7 @@ impl<'brand> Context<'brand> {
7880
pub fn new(token: GhostToken<'brand>) -> Self {
7981
Context {
8082
inner: Arc::new(Mutex::new(WithGhostToken {
81-
_token: token,
83+
token,
8284
inner: ContextInner { slab: vec![] },
8385
})),
8486
}
@@ -169,6 +171,15 @@ impl<'brand> Context<'brand> {
169171
lock.inner.slab[bound.index].shallow_clone()
170172
}
171173

174+
/// Accesses a bound through a union-bound element.
175+
pub(super) fn get_root_ref(
176+
&self,
177+
bound: &UbElement<'brand, BoundRef<'brand>>,
178+
) -> BoundRef<'brand> {
179+
let mut lock = self.lock();
180+
bound.root(&mut lock.token)
181+
}
182+
172183
/// Reassigns a bound to a different bound.
173184
///
174185
/// # Panics
@@ -195,10 +206,10 @@ impl<'brand> Context<'brand> {
195206
prod_r: &Type<'brand>,
196207
hint: &'static str,
197208
) -> Result<(), Error> {
198-
let existing_root = existing.inner.bound.root();
209+
let mut lock = self.lock();
210+
let existing_root = existing.inner.bound.root(&mut lock.token);
199211
let new_bound = Bound::Product(prod_l.inner.shallow_clone(), prod_r.inner.shallow_clone());
200212

201-
let mut lock = self.lock();
202213
lock.bind(existing_root, new_bound).map_err(|e| {
203214
let new_bound = lock.alloc_bound(e.new);
204215
drop(lock);
@@ -278,7 +289,9 @@ impl<'brand> DagLike for (&'_ Context<'brand>, BoundRef<'brand>) {
278289
match self.0.get(&self.1) {
279290
Bound::Free(..) | Bound::Complete(..) => Dag::Nullary,
280291
Bound::Sum(ref ty1, ref ty2) | Bound::Product(ref ty1, ref ty2) => {
281-
Dag::Binary((self.0, ty1.bound.root()), (self.0, ty2.bound.root()))
292+
let root1 = self.0.get_root_ref(&ty1.bound);
293+
let root2 = self.0.get_root_ref(&ty2.bound);
294+
Dag::Binary((self.0, root1), (self.0, root2))
282295
}
283296
}
284297
}
@@ -313,26 +326,29 @@ impl<'brand> ContextInner<'brand> {
313326
);
314327
self.slab[bound.index] = new;
315328
}
329+
}
316330

331+
impl<'brand> WithGhostToken<'brand, ContextInner<'brand>> {
317332
/// It is a common situation that we are pairing two types, and in the
318333
/// case that they are both complete, we want to pair the complete types.
319334
///
320335
/// This method deals with all the annoying/complicated member variable
321336
/// paths to get the actual complete data out.
322337
fn complete_pair_data(
323-
&self,
338+
&mut self,
324339
inn1: &TypeInner<'brand>,
325340
inn2: &TypeInner<'brand>,
326341
) -> Option<(Arc<Final>, Arc<Final>)> {
327-
let bound1 = &self.slab[inn1.bound.root().index];
328-
let bound2 = &self.slab[inn2.bound.root().index];
342+
let idx1 = inn1.bound.root(&mut self.token).index;
343+
let idx2 = inn2.bound.root(&mut self.token).index;
344+
let bound1 = &self.slab[idx1];
345+
let bound2 = &self.slab[idx2];
329346
if let (Bound::Complete(ref data1), Bound::Complete(ref data2)) = (bound1, bound2) {
330347
Some((Arc::clone(data1), Arc::clone(data2)))
331348
} else {
332349
None
333350
}
334351
}
335-
336352
/// Unify the type with another one.
337353
///
338354
/// Fails if the bounds on the two types are incompatible
@@ -341,9 +357,11 @@ impl<'brand> ContextInner<'brand> {
341357
existing: &TypeInner<'brand>,
342358
other: &TypeInner<'brand>,
343359
) -> Result<(), BindError<'brand>> {
344-
existing.bound.unify(&other.bound, |x_bound, y_bound| {
345-
self.bind(x_bound, self.slab[y_bound.index].shallow_clone())
346-
})
360+
existing
361+
.bound
362+
.unify(self, &other.bound, |self_, x_bound, y_bound| {
363+
self_.bind(x_bound, self_.slab[y_bound.index].shallow_clone())
364+
})
347365
}
348366

349367
fn bind(
@@ -386,8 +404,8 @@ impl<'brand> ContextInner<'brand> {
386404
Bound::Product(ref ty1, ref ty2),
387405
)
388406
| (CompleteBound::Sum(ref comp1, ref comp2), Bound::Sum(ref ty1, ref ty2)) => {
389-
let bound1 = ty1.bound.root();
390-
let bound2 = ty2.bound.root();
407+
let bound1 = ty1.bound.root(&mut self.token);
408+
let bound2 = ty2.bound.root(&mut self.token);
391409
self.bind(bound1, Bound::Complete(Arc::clone(comp1)))?;
392410
self.bind(bound2, Bound::Complete(Arc::clone(comp2)))
393411
}

src/types/mod.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ pub struct Type<'brand> {
192192
struct TypeInner<'brand> {
193193
/// A set of constraints, which maintained by the union-bound algorithm and
194194
/// is progressively tightened as type inference proceeds.
195-
bound: UbElement<BoundRef<'brand>>,
195+
bound: UbElement<'brand, BoundRef<'brand>>,
196196
}
197197

198198
impl TypeInner<'_> {
@@ -258,7 +258,9 @@ impl<'brand> Type<'brand> {
258258

259259
/// Accessor for the data of this type, if it is complete
260260
pub fn final_data(&self) -> Option<Arc<Final>> {
261-
if let Bound::Complete(ref data) = self.ctx.get(&self.inner.bound.root()) {
261+
let root = self.ctx.get_root_ref(&self.inner.bound);
262+
let bound = self.ctx.get(&root);
263+
if let Bound::Complete(ref data) = bound {
262264
Some(Arc::clone(data))
263265
} else {
264266
None
@@ -276,27 +278,27 @@ impl<'brand> Type<'brand> {
276278

277279
/// Converts a type as-is to an `Incomplete` type for use in an error.
278280
pub fn to_incomplete(&self) -> Arc<Incomplete> {
279-
let root = self.inner.bound.root();
281+
let root = self.ctx.get_root_ref(&self.inner.bound);
280282
Incomplete::from_bound_ref(&self.ctx, root, false)
281283
}
282284

283285
/// Attempts to finalize the type. Returns its TMR on success.
284286
pub fn finalize(&self) -> Result<Arc<Final>, Error> {
285-
let root = self.inner.bound.root();
287+
let root = self.ctx.get_root_ref(&self.inner.bound);
286288
let bound = self.ctx.get(&root);
287289
if let Bound::Complete(ref data) = bound {
288290
return Ok(Arc::clone(data));
289291
}
290292

291293
// First, do occurs-check to ensure that we have no infinitely sized types.
292-
if let Some(infinite_bound) = Incomplete::occurs_check(&self.ctx, root) {
294+
if let Some(infinite_bound) = Incomplete::occurs_check(&self.ctx, root.shallow_clone()) {
293295
return Err(Error::OccursCheck { infinite_bound });
294296
}
295297

296298
// Now that we know our types have finite size, we can safely use a
297299
// post-order iterator to finalize them.
298300
let mut finalized = vec![];
299-
for data in (&self.ctx, self.inner.bound.root()).post_order_iter::<NoSharing>() {
301+
for data in (&self.ctx, root).post_order_iter::<NoSharing>() {
300302
let bound_get = data.node.0.get(&data.node.1);
301303
let final_data = match bound_get {
302304
Bound::Free(_) => Final::unit(),
@@ -339,9 +341,8 @@ const MAX_DISPLAY_LENGTH: usize = 10000;
339341

340342
impl fmt::Debug for Type<'_> {
341343
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
342-
for data in (&self.ctx, self.inner.bound.root())
343-
.verbose_pre_order_iter::<NoSharing>(Some(MAX_DISPLAY_DEPTH))
344-
{
344+
let root = self.ctx.get_root_ref(&self.inner.bound);
345+
for data in (&self.ctx, root).verbose_pre_order_iter::<NoSharing>(Some(MAX_DISPLAY_DEPTH)) {
345346
if data.index > MAX_DISPLAY_LENGTH {
346347
write!(f, "... [truncated type after {} nodes]", MAX_DISPLAY_LENGTH)?;
347348
return Ok(());
@@ -376,9 +377,8 @@ impl fmt::Debug for Type<'_> {
376377

377378
impl fmt::Display for Type<'_> {
378379
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
379-
for data in (&self.ctx, self.inner.bound.root())
380-
.verbose_pre_order_iter::<NoSharing>(Some(MAX_DISPLAY_DEPTH))
381-
{
380+
let root = self.ctx.get_root_ref(&self.inner.bound);
381+
for data in (&self.ctx, root).verbose_pre_order_iter::<NoSharing>(Some(MAX_DISPLAY_DEPTH)) {
382382
if data.index > MAX_DISPLAY_LENGTH {
383383
write!(f, "... [truncated type after {} nodes]", MAX_DISPLAY_LENGTH)?;
384384
return Ok(());

0 commit comments

Comments
 (0)