Closed apoelstra closed 2 months ago
To summarize:
LockedContext
ensures we keep a write lock on the slab for the entire time of the operation. No other threads can write to the slab until we are finished.
Is that correct?Is that correct?
Correct on both counts.
Fixed both nits. Will leave the ShallowClone
stuff to a later PR.
Our existing type inference engine assumes a "global" set of type bounds, which has two bad consequences: one is that if you are constructing multiple programs, there is no way to "firewall" their type bounds so that you cannot accidentally combine type variables from one program with type variables from another. You just need to be careful. The other consequence is that if you construct infinitely sized types, which are represented as a reference cycle, the existing inference engine will leak memory.
To fix this, we need to stop allocating type bounds using untethered
Arc
s and instead use a slab allocator, which allows all bounds to be dropped at once, regardless of their circularity. This should also improve memory locality and our speed, as well as reducing the total amount of locking and potential mutex contention if type inference is done in a multithreaded context.This is a 2000-line diff but the vast majority of the changes are "API-only" stuff where I was moving types around and threading new parameters through dozens or hundreds of call sites. I did my best to break everything up into commits such that the big-diff commits don't do much of anything and the real changes happen in the small-diff ones to make review easier.
By itself, this PR does not fix the issue of reference cycles, because it includes an
Arc<Context>
inside the recursiveType
type itself. Future PRs will:bind
andunify
calls, so that these all happen atomically, including all recursive calls.Type
which eliminates theArc<Context>
and its potential for circular references. Along the way, make theBound
type private, which is not really used outside of the types module anyway.Drop
impl forArc<Node<N>>
.