[MLton] cvs commit: critical sections during thread switch

[Matthew Fluet]

> > My big question is whether or not the stack resizing in forward is correct
> > given the new invariants.
>
> Since we now ensureMutatorInvariant, which ensures
> mutatorStackInvariant, before returning to the mutator, it seems to me
> that the only invariant that we need for stacks in the heap is that
> stack->used <= stack->reserved.  So, I don't see any problem with the
> stack shrinking in forward.  In fact, with the new weaker invariant we
> can change stackNeedsReserved to return stack->used.

But we have a stronger invariant for the current thread; we can shrink
stacks on the heap down until stack->reserved == stack->used (modulo
alignment), but we can't do that for the current thread's stack.  For the
current thread, we can't drop reserved below

  stack->used + stackSlop - topFrameSize(stack)

which is precisely what stackNeedsReserved is computing.  (And, therefore,
was preserving the mutatorStackInvariant for stacks on the heap.)

So, it seems to me that for stack resizing, we ought to keep
stackNeedsReserved as is -- there's no sense in breaking the
mutatorStackInvariant on heap stacks if we can preserve it.

> Also, I think we can drop stackTopIsOk, which is now only used in
> GC_copyThread.

Agreed.

> One other thing.  I think we can drop the stackIsEmpty test in
> mutatorStackInvariant, since mutator stacks will always be nonempty
> (they will have at least the frame that called the runtime).

The mutatorStackInvariant is not true for the initial stack created by
newWorld.  So, we can skip the mutatorInvariant assert at the end of
GC_init, or push the mutatorStackInvariant check only back into the loaded
world case; or leave the stackIsEmpty test.

I'm in favor of dropping the stackIsEmpty and pushing the
mutatorStackInvariant check into the loaded world case, leaving an
invariant(s) check at the join point.


Also, I discovered that the mutatorFrontierInvariant is not maintained;
or, more accurately, the current thread's bytesNeeded is not maintained at
every entry into the runtime.  In particular, I was seeing cases where
invoking GC_copyThread used up enough of the heap that the frontier
invariant was invalidated, since the current thread's bytesNeeded went
down by the space consumed by the new thread.

I also note that since GC_switchToThread doesn't change the bytesNeeded of
the currentThread, if the current thread was saved and then resumed, then
it will attempt to get as much heap as it needed at it's last GC point,
which may be more than it needs after the switch point.