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

Use IOCP on Windows #106

Merged
merged 5 commits into from Apr 19, 2018

Conversation

Projects
None yet
2 participants
@gaborcsardi
Member

gaborcsardi commented Apr 19, 2018

Instead of WaitForMultipleObjects.

This allows polling more than 64 connections at once. It also simplifies the workflow a bit.

The polling part is not very clean, because it turns out that the connection -> pollable conversion is not a very good model. In particular, on Windows, it is hard to find the connection that responses, after a poll. The libuv model (i.e. their "inheritance" between struct types) is a better fit.

gaborcsardi added some commits Apr 19, 2018

Poll stress test, aiming on windows
This currently fails, because WaitForMultipleObjects
can wait on at most 64 objects. It will work, once
we switch to IOCP.
Win: switch to IOCP
This allows polling more than 64 connections,
and also  simplifies the workflow somewhat.
@codecov-io

This comment has been minimized.

codecov-io commented Apr 19, 2018

Codecov Report

Merging #106 into master will increase coverage by 0.28%.
The diff coverage is 95.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   74.09%   74.38%   +0.28%     
==========================================
  Files          28       29       +1     
  Lines        2471     2495      +24     
==========================================
+ Hits         1831     1856      +25     
+ Misses        640      639       -1
Impacted Files Coverage Δ
src/win/stdio.c 86.66% <ø> (ø) ⬆️
src/poll.c 100% <100%> (ø) ⬆️
src/test-connections.cpp 100% <100%> (ø) ⬆️
src/win/iocp.c 83.33% <83.33%> (ø)
src/processx-connection.c 91.9% <96.72%> (+0.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80affdc...713c3a5. Read the comment docs.

@gaborcsardi gaborcsardi merged commit 85e8fdb into master Apr 19, 2018

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@gaborcsardi gaborcsardi referenced this pull request Apr 19, 2018

Closed

Rewrite I/O using IOCP #81

@gaborcsardi gaborcsardi deleted the iocp branch May 21, 2018

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