Refactor the IOCP event loop (timers, ...)#15238
Merged
straight-shoota merged 16 commits intocrystal-lang:masterfrom Dec 6, 2024
Merged
Refactor the IOCP event loop (timers, ...)#15238straight-shoota merged 16 commits intocrystal-lang:masterfrom
straight-shoota merged 16 commits intocrystal-lang:masterfrom
Conversation
`Fiber#cancel_timeout` is a misnommer: it doesn't cancel any timeout but cancels the timeout action of a `select` expression only. It has no effect on overlapped operations.
We can dequeue more than one operation when we check for completed overlapped operations. It should be a little more efficient than returning to the evloop after each resumable fiber.
- store timers in pairing heap (EventLoop::Timers) - use high resolution timers (CreateWaitableTimer) - associate HR timer to IOCP (to interrupt blocking wait) - use post queued completion to interrupt blocking wait (instead of user APC) - thread safety for parallel timer enqueue / dequeue - overhaul of #run to make sure timeouts are cancelled (no dead fibers) Issue: the API to associate a HR timer to an IOCP requires Windows 10+ while we claim to support Windows 7+.
This allows the IOCP event loop to check for the presence of the symbols in the `ntdll.dll` library at runtime and to only use the high resolution timer on Windows 10+ and fallback to a timeout for the wait queued completion status call (and a manual interrupt when we queue a timer that expires sooner).
ysbaddaden
force-pushed
the
feature/evloop-iocp
branch
from
13de864 to
fc2476b
Compare
Collaborator
Author
|
Rebased from master & ready for review 🙇 |
Sija
reviewed
Dec 2, 2024
ysbaddaden
force-pushed
the
feature/evloop-iocp
branch
from
ec0c69f to
d2cb083
Compare
straight-shoota
approved these changes
Dec 2, 2024
This was referenced Dec 3, 2024
The IOCP event loop now always waits forever when blocking, we don't need to schedule a fiber with a long expiration anymore.
Collaborator
Author
|
Following a comment by @HertzDevil we don't need to keep a fiber with a far away expiration timer anymore on Windows. I pushed one last commit to drop it. |
straight-shoota
approved these changes
Dec 3, 2024
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrades the IOCP event loop for Windows to be on par with the Polling event loops (epoll, kqueue) on UNIX. After a few low hanging fruits (enqueue multiple fibers on each call, for example) the last commit completely rewrites the
#runmethod:CreateWaitableTimer);The main issue is that the API to associate a wait object to a completion port (seec/ntdll.cr) only goes back to Windows 10 (see this article) while we aim to be compatible with Windows 7+. Go searches the symbols at runtime and falls back to use thetimeoutparameter when the API isn't available. I guess we could do the same, making sure to interrupt when we enqueue a timer that happens sooner.I'll skip the HR timers into another PR, and implement the Windows 7+ solution for now?It was actually easy to dynamically load the symbols at runtime (thanks to @HertzDevil paving the way) so we can use the HR timers on Windows 10+ and fallback to a
timeouton legacy Windows.The spec/std_spec test suite are passing in a Windows 11 VM, in any combination of MT/no MT and legacy timeout/HR timers.
Built on top of #15226. You may skip the first commit.Rebased and ready for review.TODO after merge: add the new dependency to
ntdll.dllin the book and other places.