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

Windows bug? #70

Open
rsepassi opened this issue Sep 28, 2023 · 6 comments
Open

Windows bug? #70

rsepassi opened this issue Sep 28, 2023 · 6 comments

Comments

@rsepassi
Copy link
Contributor

rsepassi commented Sep 28, 2023

Don't have a clean repro at the moment, but initial check of the Windows support indicated that 2 timers were running sequentially instead of in parallel; could very well have been holding it wrong, but the same code worked fine on Linux and Mac. cc @Corendos

Non-clean repro is the zigcoro test. https://github.com/rsepassi/zigcoro windows branch

@Corendos
Copy link
Contributor

I don't really have a way of checking right now, but from what I get from your message (and if I'm not misunderstanding) I'd say that it's to be expected for the timer to run sequentially.

Since the event-loop is single-threaded, I don't see a way where the timers would run in parallel. Could you point me to the failing test in zigcoro ? 🙂

@rsepassi
Copy link
Contributor Author

rsepassi/zigcoro#13

zig build test-aio

Not the cleanest repro. I think a simpler repro would simply start 2 timers in parallel, e.g. 1 for 10ms and another for 20ms, wait for both to complete, and test that no more than ~20ms has passed (i.e. should be <<30ms). The event loop is single-threaded but it can have many in-flight operations going at once.

@Corendos
Copy link
Contributor

I'll try to investigate when possible.

In the meantime, I think it's important to notice that timing on Windows is really hard to get right. If I'm correct, granularity is very high. For example, multiple milliseconds can pass when using the sleep function. Maybe that's similar with the timeout given to the GetQueuedCompletionStatusEx function that I use in the code.

@rsepassi
Copy link
Contributor Author

rsepassi commented Sep 29, 2023 via email

@Corendos
Copy link
Contributor

Hey @rsepassi

I tried to reproduce the issue at home and it seems to be an issue of granularity. I had some adaptations to make due to latest Zig version.

The "aio sleep run" test is sometimes failing but multiplying the durations by 10 seems to make it work everytime, so I'd say that's more of an issue with what I mentioned earlier about Windows timing precision.

The "aio concurrent sleep" should manifest the same behavior but it's segfaulting currently so I can't be sure.

Unfortunately, I don't really know what can be done currently to improve timing precision on Windows, that was one of the limitation I mentioned in the PR 😕

I tested with Zig 0.12.0-dev.1261+bb0419599

@amoldeshpande
Copy link

amoldeshpande commented Apr 19, 2024

The way to do higher resolution sleeps/waits in Windows is the multimedia timer API.

This blog post explains a recent change to the framework and gives good information about timer resolutions in the process.

https://randomascii.wordpress.com/2020/10/04/windows-timer-resolution-the-great-rule-change/

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

No branches or pull requests

3 participants