Skip to content

[Windows][Concurrency] Use the same clock as Dispatch. #76310

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

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Sep 6, 2024

The Concurrency runtime calculates deadlines for scheduling itself using swift_get_time(); unfortunately, on Windows that was using QueryPerformanceCounter(), while Dispatch uses
QueryInterruptTimePrecise(). The problem with that is that the two do not necessarily correspond at all. In general
QueryPerformanceCounter() may be using any of a number of hardware timers depending on the machine on which we're running.

In the VM I was testing on, the two differed by 20ms, but the worst case is that they are completely unrelated, in which case Task.sleep() will wait essentially a random amount of time.

Fixes #72095.

rdar://135413803

The Concurrency runtime calculates deadlines for scheduling itself
using `swift_get_time()`; unfortunately, on Windows that was using
`QueryPerformanceCounter()`, while Dispatch uses
`QueryInterruptTimePrecise()`.  The problem with that is that the two do
not necessarily correspond *at all*.  In general
`QueryPerformanceCounter()` may be using any of a number of hardware
timers depending on the machine on which we're running.

In the VM I was testing on, the two differed by 20ms, but the worst case
is that they are completely unrelated, in which case `Task.sleep()` will
wait essentially a random amount of time.

rdar://135413803
@al45tair al45tair requested a review from compnerd September 6, 2024 14:39
@al45tair al45tair requested a review from ktoso as a code owner September 6, 2024 14:39
@al45tair
Copy link
Contributor Author

al45tair commented Sep 6, 2024

@swift-ci Please test

@al45tair al45tair requested a review from grynspan September 6, 2024 14:41
Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to update swift_get_clock_res().

@al45tair
Copy link
Contributor Author

al45tair commented Sep 6, 2024

You also need to update swift_get_clock_res().

Nice spot. You're right, I do.

@grynspan
Copy link
Contributor

grynspan commented Sep 6, 2024

You also need to update swift_get_clock_res().

Nice spot. You're right, I do.

Fortunately, you can just use the exact same code as for the other clock. :)

I should have updated this to match as well.

rdar://135413803
@al45tair
Copy link
Contributor Author

al45tair commented Sep 6, 2024

@swift-ci Please test

Since we're linking `mincore.lib`, we don't need to use
`GetProcAddress()` to find `Query[Unbiased]InterruptTimePrecise()`.

rdar://135413803
@al45tair
Copy link
Contributor Author

al45tair commented Sep 6, 2024

@swift-ci Please test

@al45tair al45tair merged commit e34d23f into swiftlang:main Sep 7, 2024
5 checks passed
@grynspan
Copy link
Contributor

Related: #63225

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

Successfully merging this pull request may close these issues.

Task.sleep() sleep too long on Windows
4 participants