[MLton] Int.fmt thread unsafe? (YUCK)

[Stephen Weeks]

> By putting the conditional call to more inside the wind you have the
> advantage that the critical region is smaller (it does not include
> more), but the disadvantage that you have to test b again.

I don't think it's putting the call to more inside the wind that makes
the critical region smaller.  Rather, it's noticing that the test and
set of staticIsInUse is all that needs to be protected.

> One can eliminate all but one test using the following (rather ugly) code:
> 
>     fun use (T {more, static, staticIsInUse}, f) =
>            (Primitive.Thread.atomicBegin ()
>             ; if ! staticIsInUse
>                  then (Primitive.Thread.atomicEnd ()
>                        ; f (more ()))
>                  else (staticIsInUse := true
>                        ; Primitive.Thread.atomicEnd ()
>                        ; DynamicWind.wind (fn () => f static,
>                                            fn () => staticIsInUse := false)))

Here's some code that's half way between yours and mine.  Duplicating
the atomicEnd across the second conditional leads to yours.  Pushing
the second conditional into the wind leads to mine.

      fun use (T {more, static, staticIsInUse}, f) =
         let
            val () = Primitive.Thread.atomicBegin ()
            val b = !staticIsInUse
            val () = if b then () else staticIsInUse := true
            val () = Primitive.Thread.atomicEnd ()
         in
            if b then
               f (more ())
            else
               DynamicWind.wind (fn () => f static,
                                 fn () => staticIsInUse := false)
         end

> Note, the code for f is now called in two places (with different
> continuations) and that might change inlining.

I think this is the key difference.  And not duplicating f might mean
an extra procedure call in the common case, which is almost certainly
worse than two extra tests, or duplicated code.  So, it think the
original code I sent is best, despite the extra two tests.  Also,
don't forget that atomicBegin and atomicEnd each have their own test
that will be performed in any case.  We used to have an optimization
that got rid of them in single-threaded code, but we eliminated it a
few years back because of correctness issues -- atomicBegin and
atomicEnd also manipulate a mutable int (canHandle) whose value is
checked even in single threaded code.