Working on issue #15 (work in progress)

[thomasjm]

This is some progress I've made on issue #15. It doesn't solve the problem yet but I'm putting it up now to gather feedback/advice.

I've been trying to follow the approach of the process library to make the fork call inside posix-pty play nicely with the GHC RTS. See how blockUserSignals/unblockUserSignals and stopTimer/startTimer are used here: https://github.com/haskell/process/blob/master/cbits/runProcess.c#L137

I think adding unsafe to the foreign import ccall lines is a good idea no matter what; see http://wiki.haskell.org/GHC/Using_the_FFI#Improving_efficiency

[thomasjm]

Duplicating a comment I left on Reddit:

I tried to mimic the approach of process by blocking user signals and stopping the timer when the fork happens. However, I think there's a basic problem here. System.Process.Posix contains the following comment:

-- runInteractiveProcess() blocks signals around the fork(). 
-- Since blocking/unblocking of signals is a global state 
-- operation, we better ensure mutual exclusion of calls to 
-- runInteractiveProcess().

System.Process.Posix contains a lock to facilitate this which it takes before calling into its C code. That's all well and good for System.Process, but how am I supposed to ensure mutual exclusion between System.Process calls and System.Posix.Pty calls?

It seems like it might be impossible for a library to safely do tricky things like fork because you have to coordinate with global changes made by other libraries like System.Process.

[merijn]

Hey, thanks for looking into this. Just a heads up, I'm rather swamped at work right now, so I won't really have time to dive into and evaluate this until a few weeks from now.

[thomasjm]

Sounds good, thanks @merijn . FYI after the last couple commits the fix seems to be working. This fix depends on haskell/process#154 landing.

[thomasjm]

@merijn I've been using this for a while without issue, and the necessary change in process has been merged for a while. How about merging this?

[merijn]

I wanna check out what some of the RTS functions actually do so I understand what I merge, but it seems fine. I probably won't get around to making a new release until somewhere next week, though.

[thomasjm]

Friendly ping here @merijn

[merijn]

Thanks for the ping, I got distracted. There's a few redundant imports (I can remove those to, if that's easier). The one thing I'm suspicious about is changing the foreign imports to unsafe. What's the reason for that?

[merijn]

Is there any specific reason these imports should be unsafe?

[thomasjm]

Yes, for performance. Foreign code that's not expected to a) block or b) call back into Haskell code can be marked unsafe to reduce overhead; see https://wiki.haskell.org/Foreign_Function_Interface#Unsafe_calls

[merijn]

What they forget to mention is that, even for non-blocking functions unsafe can be rather problematic if the function is slow as for the duration of the foreign call the GC is blocked. So if GC is triggered all threads will be blocked and waiting idle for the duration of the foreign call.

So I'd be hesitant to change that, especially for foreign code making syscalls (like these) since the overhead of a safe call is tiny compared to the overhead of making a syscall.

[thomasjm]

the overhead of a safe call is tiny compared to the overhead of making a syscall

That's not what I thought, do you have a citation? From what I can google the overhead of safe is pretty significant and can make the overall call 10x slower; see here. Quoting Simon Marlow: "safe calls are more expensive and involve taking and releasing locks, and possibly creating a new OS thread." I'm sure it depends on the syscall as well. For the relatively simple calls like get/set_pty_size I'd expect unsafe to be a win.

So if GC is triggered all threads will be blocked and waiting idle for the duration of the foreign call.

Not sure what you mean by this -- I believe GHC will simply not trigger GC while an unsafe call is active, so there's no scenario where other capabilities not involved in the unsafe call get blocked. Given the syscall duration is probably measured in 10s/100s of nanoseconds and typical GC frequency is measured in 100s of milliseconds it should not be a problem.

[merijn]

Of course safe calls are more expensive. The question is "does that matter?". You're kinda glossing over the takeaway right at the top of Edward's post :)

Don’t use unsafe in your FFI imports! We really mean it!

Anyway, I'm fairly familiar with the low level details of safe vs unsafe calls and I'm not really inclined to mark foreign calls unsafe unless I'm 100% it's 1) necessary and 2) safe. So, in the absence of data showing a problem I'll keep them as is. I'll merge this without them, do a bit of local testing and then make a new release on Hackage.

[thomasjm]

You're kinda glossing over the takeaway right at the top of Edward's post

Not really, he acknowledges in the comments that the C code he's dealing with is doing intensive things like solving SAT problems. I still don't think this rule applies to a short C function with a single syscall / don't get why you'd rather trade off a guaranteed performance hit on every call to avoid delaying a GC iteration by a tiny amount (unless I'm wrong about the "blocking every thread" scenario?). But agreed it's unlikely to cause a noticeable problem.

Thanks for merging!