Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP : Basic WinIO support #509

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP : Basic WinIO support #509

wants to merge 2 commits into from

Conversation

Mistuke
Copy link
Collaborator

@Mistuke Mistuke commented Jul 31, 2021

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 Mistuke self-assigned this Jul 31, 2021
@Mistuke Mistuke changed the title WIP: Basic WinIO support WIP : Basic WinIO support Jul 31, 2021
@kazu-yamamoto
Copy link
Collaborator

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

@kazu-yamamoto
Copy link
Collaborator

@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.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Feb 15, 2022

@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!

@kazu-yamamoto
Copy link
Collaborator

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.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Feb 17, 2022

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

@kazu-yamamoto
Copy link
Collaborator

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?

@Mistuke
Copy link
Collaborator Author

Mistuke commented Feb 17, 2022

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.

@kazu-yamamoto
Copy link
Collaborator

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

@Mistuke
Copy link
Collaborator Author

Mistuke commented Feb 17, 2022

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
Copy link
Collaborator Author

Mistuke commented Feb 17, 2022

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
Copy link
Collaborator Author

Mistuke commented Feb 20, 2022

@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.

@kazu-yamamoto
Copy link
Collaborator

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants