Add sleep support to win32 event loop #10605
Conversation
straight-shoota
added
kind:feature
topic:stdlib:concurrency
platform:windows
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
spec/std/concurrent_spec.cr
Outdated
| chan = Channel(Int32).new | ||
| spawn do | ||
| 3.times do |i| | ||
| sleep 40.milliseconds | ||
| chan.send (i + 1) | ||
| end | ||
| end | ||
| spawn do | ||
| 2.times do |i| | ||
| sleep 100.milliseconds | ||
| chan.send (i + 1) * 10 | ||
| end | ||
| end | ||
|
|
||
| Array(Int32).new(5) { chan.receive }.should eq [1, 2, 10, 3, 20] |
There was a problem hiding this comment.
This newly added spec fails on darwin. I've already increased the sleep times trying to avoid inconsistencies.
Could someone run this example on macOS to find a sleep amount that leads to success consistently? Or investigate why if it continues to fail? :D
There was a problem hiding this comment.
Mmm... We need this in order to get the final approval, otherwise the macOS test will start failing on master.
There was a problem hiding this comment.
Do we know why it fails? Those times indicate some pretty insane overhead.
There was a problem hiding this comment.
I don't know why. We need someone to test this on macOS manually.
But I'll disable this spec on darwin for now, to keep CI happy.
| sleep_time = next_event.wake_at - Time.monotonic | ||
|
|
||
| if sleep_time > Time::Span.zero | ||
| LibC.Sleep(sleep_time.total_milliseconds) |
There was a problem hiding this comment.
Hmm. This can only work when all events are sleeps, right?
Otherwise a completion could happen but not be received until the sleep is over. I think the fetch completion command has a timeout that can be used for the case when there is both completions and sleeps happening. But perhaps actual sleep would still be necessary in the case where there are no completions, that depends on how the fetching command behave if empty.
There was a problem hiding this comment.
Yes, this will later be changed to GetQueuedCompletionStatus. See comment in the OP. For now, Sleep is good enough until we can introduce completion ports properly (this is a preparatory step for that).
This patch enhances the event loop for win32 to support sleeping fibers.
The implementation relies on
Sleepfor now. It will be replaced by a timeout withGetQueuedCompletionStatusin a subsequent step (see #9957 (comment)).