[MLton] cvs commit: MAIL: PosixError.SysCall changes

[Stephen Weeks]

> I wanted to raise a question as to the correct behavior of
> Posix.Process.pause.  Before this checkin, the code looked like:
>   fun pause () = Error.checkResult (Prim.pause ())
> Note that this would _always_ raise SysError, because pause is supposed to
> return -1 with errno set to EINTR.  Under the new code, we raise an error
> if errno is anything but EINTR.  This seems to be better behavior to me.

Definitely.  I was probably just slavishly following the idea that a
~1 return value should raise SysErr.

> > OS.IO.poll
> > 	It seems like the array read after the call should be in the
> > 	critical section.
> 
> I don't think it needs to be in the critical section.  The reventss array
> is local to the poll function, so there is no contention; 

Ahh.  OK.

> And, in my mind, the less code in the critical section, the better.

Agreed.

> > One general worry I have is the use of {restart = false} to optimize
> > system calls which are believed to never return EINTR.
...
> I haven't done any benchmarks or anything, so I don't know if there is an
> appreciable slowdown in the new code.  I strongly doubt it, as in the
> common case, we are only doing one additional comparison.

I understand this to mean that the additional comparison is "errno =
intr", which I agree is cheap.  Hence, I don't see the benefit of the
"restart andalso" optimization to eliminate this test.

> And I got a major speed up in CML by only wrapping print with
> atomicBegin/atomicEnd rather than block/unblock.

This makes perfect sense, since atomic{Begin,End} are a few
instructions, while block/unblock are system calls.  But I don't see
the relevance to the restart optimization.