Rewrite Windows backend to use IOCP exclusively #1269
Conversation
Also: - Increase test timeout, because for some reason the "PCTools" LSP is sometimes incredibly slow to install. - Factor out common curl options in ci.sh, for more consistency/reliability - Run cleanup/codecov upload even if tests failed
As of this commit, you can call trio._core._generated_io_windows.afd_poll and it seems to basically work.
This is a nice solid implementation *except* that it turns out I had a fundamental misunderstanding of how AFD_POLL works, so it all falls apart as soon as you have multiple tasks waiting on the same socket simultaneously.
Codecov Report
@@ Coverage Diff @@
## master #1269 +/- ##
==========================================
+ Coverage 99.57% 99.69% +0.11%
==========================================
Files 106 105 -1
Lines 12852 12945 +93
Branches 987 997 +10
==========================================
+ Hits 12797 12905 +108
+ Misses 35 23 -12
+ Partials 20 17 -3
|
This should improve coverage on Windows backend
No need for gratuitous inconsistencies.
Mostly to convince myself I wasn't imagining it.
|
OK, I think I've addressed all the comments and all the other cleanups I noticed, so this is ready for review again. |
| # - number of bytes transferred (named "InternalHigh"); redundant with | ||
| # dwNumberOfBytesTransferred *if* this is a regular I/O event. | ||
| # | ||
| # The general model is that you call some function like ReadFile or WriteFile |
There was a problem hiding this comment.
Thank you for updating this comment, it's super helpful!
| @@ -0,0 +1,173 @@ | |||
| # A little script to experiment with AFD polling. | |||
There was a problem hiding this comment.
Is it possible for the IOCP-based I/O manager implemented in this PR to run into the bug elicited by this script? If not, maybe comment why not?
There was a problem hiding this comment.
Right, this bug is why we have all this machinery to avoid ever issuing more than one IOCTL_AFD_POLL operation on the same socket at the same time :-). Updated comments in both afd-lab.py and _io_windows.py.
|
Test failure is codecov flakiness (#1231) |
|
There was some discussion of scaling problems with It looks like the conclusion for now is that this PR is still an improvement over what we had before, but we probably want a follow-up issue to consider splitting up our AFD_POLL operations across multiple AFD helper handles, to improve scalability even more. I did add the script that demonstrates this to |
|
OK, let's see how this goes! Thanks everyone for your comments and feedback so far! |
This dramatically simplifies the code (no more janky background
thread!), and probably makes it substantially faster (though I haven't
measured), since we're not using
selectanymore or passing eventsback and forth between threads.
The way we accomplish this is basically to reimplement
selectourselves, using the undocumented
IOCTL_AFD_POLLmechanism to talkto the kernel directly. Other projects that use a similar approach are
libuv, wepoll, and mio. See #52 for (lots) more discussion.
Since this is undocumented, it's hard to be 100% certain we've
understood everything, but let's see what the tests say :-). I also
added some tests that install "layered service providers", which are
this weird system on Windows that lets random libraries monkeypatch
how sockets work; I think we're doing all the stuff we need to to be
compatible with these things, but again, we need to check that
empirically.
This also unblocks the idea of allowing Trio to run "over" a foreign
event loop, as discussed in #399 (comment)