Skip to content

Commit 060455a

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 671a836 commit 060455a

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
}
@@ -151,6 +153,15 @@ impl<'brand> Context<'brand> {
151153
lock.inner.slab[bound.index].shallow_clone()
152154
}
153155

156+
/// Accesses a bound through a union-bound element.
157+
pub(super) fn get_root_ref(
158+
&self,
159+
bound: &UbElement<'brand, BoundRef<'brand>>,
160+
) -> BoundRef<'brand> {
161+
let mut lock = self.lock();
162+
bound.root(&mut lock.token)
163+
}
164+
154165
/// Reassigns a bound to a different bound.
155166
///
156167
/// # Panics
@@ -177,10 +188,10 @@ impl<'brand> Context<'brand> {
177188
prod_r: &Type<'brand>,
178189
hint: &'static str,
179190
) -> Result<(), Error> {
180-
let existing_root = existing.inner.bound.root();
191+
let mut lock = self.lock();
192+
let existing_root = existing.inner.bound.root(&mut lock.token);
181193
let new_bound = Bound::Product(prod_l.inner.shallow_clone(), prod_r.inner.shallow_clone());
182194

183-
let mut lock = self.lock();
184195
lock.bind(existing_root, new_bound).map_err(|e| {
185196
let new_bound = lock.alloc_bound(e.new);
186197
drop(lock);
@@ -260,7 +271,9 @@ impl<'brand> DagLike for (&'_ Context<'brand>, BoundRef<'brand>) {
260271
match self.0.get(&self.1) {
261272
Bound::Free(..) | Bound::Complete(..) => Dag::Nullary,
262273
Bound::Sum(ref ty1, ref ty2) | Bound::Product(ref ty1, ref ty2) => {
263-
Dag::Binary((self.0, ty1.bound.root()), (self.0, ty2.bound.root()))
274+
let root1 = self.0.get_root_ref(&ty1.bound);
275+
let root2 = self.0.get_root_ref(&ty2.bound);
276+
Dag::Binary((self.0, root1), (self.0, root2))
264277
}
265278
}
266279
}
@@ -295,26 +308,29 @@ impl<'brand> ContextInner<'brand> {
295308
);
296309
self.slab[bound.index] = new;
297310
}
311+
}
298312

313+
impl<'brand> WithGhostToken<'brand, ContextInner<'brand>> {
299314
/// It is a common situation that we are pairing two types, and in the
300315
/// case that they are both complete, we want to pair the complete types.
301316
///
302317
/// This method deals with all the annoying/complicated member variable
303318
/// paths to get the actual complete data out.
304319
fn complete_pair_data(
305-
&self,
320+
&mut self,
306321
inn1: &TypeInner<'brand>,
307322
inn2: &TypeInner<'brand>,
308323
) -> Option<(Arc<Final>, Arc<Final>)> {
309-
let bound1 = &self.slab[inn1.bound.root().index];
310-
let bound2 = &self.slab[inn2.bound.root().index];
324+
let idx1 = inn1.bound.root(&mut self.token).index;
325+
let idx2 = inn2.bound.root(&mut self.token).index;
326+
let bound1 = &self.slab[idx1];
327+
let bound2 = &self.slab[idx2];
311328
if let (Bound::Complete(ref data1), Bound::Complete(ref data2)) = (bound1, bound2) {
312329
Some((Arc::clone(data1), Arc::clone(data2)))
313330
} else {
314331
None
315332
}
316333
}
317-
318334
/// Unify the type with another one.
319335
///
320336
/// Fails if the bounds on the two types are incompatible
@@ -323,9 +339,11 @@ impl<'brand> ContextInner<'brand> {
323339
existing: &TypeInner<'brand>,
324340
other: &TypeInner<'brand>,
325341
) -> Result<(), BindError<'brand>> {
326-
existing.bound.unify(&other.bound, |x_bound, y_bound| {
327-
self.bind(x_bound, self.slab[y_bound.index].shallow_clone())
328-
})
342+
existing
343+
.bound
344+
.unify(self, &other.bound, |self_, x_bound, y_bound| {
345+
self_.bind(x_bound, self_.slab[y_bound.index].shallow_clone())
346+
})
329347
}
330348

331349
fn bind(
@@ -368,8 +386,8 @@ impl<'brand> ContextInner<'brand> {
368386
Bound::Product(ref ty1, ref ty2),
369387
)
370388
| (CompleteBound::Sum(ref comp1, ref comp2), Bound::Sum(ref ty1, ref ty2)) => {
371-
let bound1 = ty1.bound.root();
372-
let bound2 = ty2.bound.root();
389+
let bound1 = ty1.bound.root(&mut self.token);
390+
let bound2 = ty2.bound.root(&mut self.token);
373391
self.bind(bound1, Bound::Complete(Arc::clone(comp1)))?;
374392
self.bind(bound2, Bound::Complete(Arc::clone(comp2)))
375393
}

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)
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)