network icon indicating copy to clipboard operation
network copied to clipboard

WIP : Basic WinIO support

Open Mistuke opened this issue 3 years ago • 12 comments

This is a work in progress to add basic WinIO support to network.

Basic here means that we're not taking full advantage of the new I/O manager but that we provide a compatibility layer between the current structures of network and WinIO.

This will allow us to progress WinIO to the next stage in GHC and turn it on by default while the discussions on an async network design are in progress.

See #364

/cc @eborden and @kazu-yamamoto , just so you know about it but it's not ready for review.

Mistuke avatar Jul 31 '21 15:07 Mistuke

@Mistuke When it gets ready, please let me know.

kazu-yamamoto avatar Aug 02 '21 06:08 kazu-yamamoto

@Mistuke I finally installed Windows 10 on VirtualBox in macOS. At this moment, my quic library does not work well since killThread to a blocked lightweight thread hangs on Windows (, right?). Also, the timer management of GHC on Windows is poor.

I'm now quite motivated to help you. Please tell me what I can do for the new async interface.

kazu-yamamoto avatar Feb 15 '22 08:02 kazu-yamamoto

@Mistuke I finally installed Windows 10 on VirtualBox in macOS. At this moment, my quic library does not work well since killThread to a blocked lightweight thread hangs on Windows (, right?). Also, the timer management of GHC on Windows is poor.

@kazu-yamamoto ah great! We're trying to get it on by default for 9.4 so network was indeed on my list of high priority tasks.

killThread itself doesn't hang (that I know off) how any exception on the thread causes all I/O to be cancelled since the I/Os are bound to the underlying OS thread for the I/O manager (on the old manager).

I'm now quite motivated to help you. Please tell me what I can do for the new async interface.

Great, I have been looking how to best support this, I'll post some of the generic things that need changing and we can go from there. I'll post them friday.

Thanks!

Mistuke avatar Feb 15 '22 21:02 Mistuke

killThread itself doesn't hang (that I know off) how any exception on the thread causes all I/O to be cancelled since the I/Os are bound to the underlying OS thread for the I/O manager (on the old manager).

@Mistuke I confirmed that such syscalls are safe on Windows but the following code compiled with -threaded hangs:

https://gist.githubusercontent.com/kazu-yamamoto/a82db7d4c5bf4609be542ab8bffce148/raw/ca454eaa0113cc1bd06cf04cfa3c05b4e3946a77/test%2520for%2520windows

I wonder why.

kazu-yamamoto avatar Feb 17 '22 00:02 kazu-yamamoto

killThread itself doesn't hang (that I know off) how any exception on the thread causes all I/O to be cancelled since the I/Os are bound to the underlying OS thread for the I/O manager (on the old manager).

@Mistuke I confirmed that such syscalls are safe on Windows but the following code compiled with -threaded hangs:

https://gist.githubusercontent.com/kazu-yamamoto/a82db7d4c5bf4609be542ab8bffce148/raw/ca454eaa0113cc1bd06cf04cfa3c05b4e3946a77/test%2520for%2520windows

I wonder why.

Because in the old I/O manager as I mentioned the operations are performed on the same underlying OS thread. That the I/O operation operation is marked safe doesn't really matter as it's the I/O manager blocking in an uninterruptable state. See https://gitlab.haskell.org/ghc/ghc/-/issues/3937

Mistuke avatar Feb 17 '22 01:02 Mistuke

OK. I misunderstood the case of MIO on Windows.

This code still hangs with --io-manager=native with GHC 9.0.2 on Windows because network is not integrated with WinIO yet, right?

kazu-yamamoto avatar Feb 17 '22 02:02 kazu-yamamoto

OK. I misunderstood the case of MIO on Windows.

This code still hangs with --io-manager=native with GHC 9.0.2 on Windows because network is not integrated with WinIO yet, right?

The code should have crashed, network at the moment cannot work with WinIO because WinIO can't deal with fd based handles.

I'm surprised that it didn't trigger the fail messages. GHC 9.0.2 also contains bugs that are fixed in later versions that are being back ported. So I'd recommend 9.2.2 for any experiments.

But yes network doesn't work at all. GHC won't be able to do anything with the Handles returned by network.

Mistuke avatar Feb 17 '22 03:02 Mistuke

Thank you for explanation. I'm waiting for GHC 9.2.2!

kazu-yamamoto avatar Feb 17 '22 03:02 kazu-yamamoto

As I mentioned I'll write it up tomorrow in a bit more detail, but to give an idea the major difference between what network does now and what winio requires is that network should never issue a blocking call at all.

any operation needs to be associated with the I/O manager by registering the handle to the completion port of the I/O manager (See e.g. https://github.com/ghc/ghc/blob/378c0bba7d132a89dd9c35374b7b4bb5a4730bf7/libraries/base/GHC/IO/Windows/Handle.hsc#L830 for how files are operations are associated), after that network must always use async operations. i.e. no more fread etc.

If a call is to be waited upon, e.g. a recv then Network must ask the io manager to do so for it, rather than doing so itself in a foreign call.

For instance see how blocking reads are handled for files https://github.com/ghc/ghc/blob/378c0bba7d132a89dd9c35374b7b4bb5a4730bf7/libraries/base/GHC/IO/Windows/Handle.hsc#L432

You have to register a callback to a special MVar which the I/O manager will invoke to give you the result of the operation. At which time you have to tell it what the result means and how you want it to proceed.

You also have to tell it how to start the I/O operation you want to perform, none of these calls are allowed to block on a foreign call.

If you're interested in how the I/O manager deals with requests you can read the notes at https://github.com/ghc/ghc/blob/f115c382a0b0cc9299e73bcb8a11ad97c5fe7c57/libraries/base/GHC/Event/Windows.hsc#L133 https://github.com/ghc/ghc/blob/f115c382a0b0cc9299e73bcb8a11ad97c5fe7c57/libraries/base/GHC/Event/Windows.hsc#L463 https://github.com/ghc/ghc/blob/f115c382a0b0cc9299e73bcb8a11ad97c5fe7c57/libraries/base/GHC/Event/Windows.hsc#L498

which will give you the general overview.

Mistuke avatar Feb 17 '22 03:02 Mistuke

It's also important to note that the design of the I/O manager safely allows any package to implement their own result handling routine. So e.g. network is allowed to implement it's own https://github.com/ghc/ghc/blob/f115c382a0b0cc9299e73bcb8a11ad97c5fe7c57/libraries/base/GHC/Event/Windows.hsc#L520 which in the future might be required to support RIO (the high throughput DMA based networking stuff in Windows).

For now though withOverlappedEx should be enough to get network going. We also export all the (haskell) helpers from GHC to make it easier to use the new I/O manager.

Mistuke avatar Feb 17 '22 03:02 Mistuke

@kazu-yamamoto Did you get a chance to check out the links above?

So essentially there are two big changes required in network:

  1. At the moment the Socket type is described as
data Socket = Socket !(IORef CInt) !CInt {- for Show -}

i.e. there's an implicit assumption that the Socket handle fits in an CInt. This will no longer be true as WinIO uses the native HANDLE type which is a pointer.

As such I think we need something like

data Socket = Socket !(IORef CInt) !CInt {- for Show -}
                   | WinSock !(IORef HANDLE) !HANDLE

The problem is that you can switch between mio and winio dynamically by giving the compiler the flag +RTS --io-manager=native. The problem then is with all the helper functions like fdSocket, unsafeFdSocket etc.

I guess there's no way around it and we need new helpers to deal with the HANDLE part. WinIO deals with which I/O manager is active by using a helper (<!>) to select between the two. so a <!> b will run a when the old I/O manager is enabled and b for the new one. Eventually we'll remove the old one and make (<|>) flip const which will optimize the calls away.

Do you agree with just adding the second constructor?

  1. Unlesss I'm mistaken, all actual Socket I/O seems to be done through Network.Socket.Buffer right? It seems to currently call back into GHC to use it's low level readRawBufferPtr. It looks like this function is using threadWaitRead and filling the given pointer directly.

It looks like the we should either handle this internally with WSARecv directly or call hwndRead to do the work, However hwndRead does not currently handle Socket status codes. So I think using WSARecv and friends directly is a) Simpler. b) More efficient c) More backwards and forwards compatible, it decouples Network from GHC's internals.

I think most changes are fairly straight forward aside from the datatype change.

Mistuke avatar Feb 20 '22 14:02 Mistuke

Did you get a chance to check out the links above?

Yes!

As such I think we need something like

data Socket = Socket !(IORef CInt) !CInt {- for Show -}
                   | WinSock !(IORef HANDLE) !HANDLE

This is exactly the same as my guess. :-)

The problem is that you can switch between mio and winio dynamically by giving the compiler the flag +RTS --io-manager=native. The problem then is with all the helper functions like fdSocket, unsafeFdSocket etc.

Oh. I'm missing this point.

I guess there's no way around it and we need new helpers to deal with the HANDLE part. WinIO deals with which I/O manager is active by using a helper (<!>) to select between the two. so a <!> b will run a when the old I/O manager is enabled and b for the new one. Eventually we'll remove the old one and make (<|>) flip const which will optimize the calls away.

I don't have strong opinion on this. We might want to chose the appoarch of building flag. But let's try <!> first.

Do you agree with just adding the second constructor?

If this refers to WinSock above, the answer is big YES.

  1. Unlesss I'm mistaken, all actual Socket I/O seems to be done through Network.Socket.Buffer right?

I think so.

I think most changes are fairly straight forward aside from the datatype change.

Sounds nice!

kazu-yamamoto avatar Feb 20 '22 23:02 kazu-yamamoto