-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
runtime: windows system clock resolution issue #8687
Comments
I confirm what DefiaQ said. The problem is that we call timeBeginPeriod Windows API. It was introduced as part of pprof implementation changeset: 9786:9c5c0cbadb4d user: Hector Chu <hectorchu@gmail.com> date: Sat Sep 17 17:57:59 2011 +1000 summary: runtime: implement pprof support for windows but I view it as "windows timer precision is very very low (about 15ms) comparing to other OSes, so lets get best precision whenever we can (1ms)". timeBeginPeriod has many problems. It does not work on some systems (it does not work on my oldish windows xp). I suspect, it fails, if you don't have correct access rights on the system (it works for administrator). When successfully set timeBeginPeriod will drain laptop batteries faster (I read about it everywhere, I didn't have chance to experience it on practice). But, I suspect, many other applications call timeBeginPeriod too, so Go is not the only one at fault here. Personally I don't care if timeBeginPeriod is set, but others might feel different. Perhaps there is case here for some way to control it from within Go program. ALternatively, people who care, can just remove timeBeginPeriod call in their own runtime. Alex |
Too late for 1.4. Maybe for a future release, if someone wants to do the work to prove that it matters and that removing this is safe. I think without a clear report about bad implications we won't do that work ourselves. Labels changed: added release-none, removed release-go1.4. Status changed to Accepted. |
i made a package to call timeEndPeriod to reset the timer to normal on initialization. if the program doesn't need much performance, but runs on a windows laptop for a long time, just import the package. |
I've done some benchmarks today. Then I ran this benchmark Then I wrote a http benchmark here So I guess, if your program don't use much IO (cause less context switches of goroutine), you can discard the system clock resoluton change. |
Could we use QueryPerformanceCounter for pprof support? |
And how are you going to use QueryPerformanceCounter in pprof support? Alex |
cyclesPerSecond uses systime in os1_windows.go https://github.com/golang/go/blob/master/src/runtime/os1_windows.go#L314-L316 https://github.com/golang/go/blob/master/src/runtime/os1_windows.go#L286-L306 This part is possible to be replaced with QueryUnbiasedInterruptTime (not QueryPerformanceCounter). |
I've blogged about this issue in the past (https://randomascii.wordpress.com/2013/07/08/windows-timer-resolution-megawatts-wasted/) and I wanted to share what I have learned: As defia pointed out, having a higher timer frequency wastes CPU time and can make some CPU heavy programs run more slowly. This is because the OS is waking up ~15 times more frequently, consuming CPU time each time it does. It also means that other programs may wake up more frequently and waste CPU time - some software sits in Sleep(1) loops and will wake up ~15 times more frequently when the timer resolution is changed. So, raising the timer frequency can waste performance. Raising the timer frequency will almost always waste some power and will harm battery life. This is the kicker. This is why no program should ever raise the timer frequency unless it needs the timer frequency raised. The timer frequency is a global setting and raising it without a good reason is poor citizenship. If you have a work loop that relies on Sleep(1) or timeouts from WaitForSingleObject(handle, 1) then raising the timer frequency will make your loop run faster. Presumably that is why the http test ran faster. Most software should not be relying on timeouts, it should be running based on events. But, it is reasonable for specific go programs to raise the timer frequency if needed. It is not reasonable for the go runtime to unilaterally raise timer frequency. |
With the background information out of the way... I discovered this issue because I noticed (using clockres.exe) that the timer frequency on my computer was always raised. I used "powercfg /energy /duration 5" to identify the culprit, a go program that was running in the background. This go program was monitoring system status and was idle about 99.9% of the time but because of the go runtime's decision to raise the timer frequency for all go programs this background program was measurably affecting power consumption and battery life. This 99.9% idle go program was affecting my computer more than a program that was actively running. This was clearly a case where the program did not need a higher timer frequency. Raising the timer frequency is a reasonable option for the go runtime, but it is absolutely not a reasonable default for the go runtime. Chrome has been fixed to not leave the timer frequency high, so has SDL, and so have many other products. Go needs to do likewise. Please. |
I appreciate and share your concerns. So I tried to measure the impact of removing timeBeginPeriod call on my windows 7 amd64 computer. (see bench.txt in https://gist.github.com/alexbrainman/914885b57518cb4a6e39 ). Unfortunately I can see quite significant slowdowns here and there. The change looks like a no go for me. Maybe gophers who know more about Go scheduler can explain the slowdowns and suggest ways to improve the situation. For the moment, if someone cares about this, they can comment out call to timeBeginPeriod. Alex |
I know that some video applications (games, Chrome, a few others) have to leave the frequency high in order to accurately schedule tasks and meet frame rates. For other programs I don't understand the need to have the timer frequency high. I would imagine that tasks would typically be happening as quickly as possible (reading files, doing calculations, passing messages) and I'm curious as to what sort of timeouts, or waits, or sleeps are being affected by the timer frequency. Anyway, if some Go programs need the timer frequency then those Go programs should raise the timer frequency. I have no problem with that. But if the Go runtime raises the timer frequency then that is a problem. We could debate whether raising the timer frequency should be opt-in (my strong vote) or opt-out, but it should be a choice. Otherwise long-running low-activity Go programs will be blacklisted by anybody who wants good battery life. Go deserves better than that. |
Things are never simple, it is always about trade-offs. But I don't know enough about Go runtime to give you explanation. And I am hopeful that other gophers will comment. All I can say is that Windows makes the task much harder when everything is measured in 15 milliseconds (comparing to nanoseconds on linux) - imagine running scheduler that performs equally well on both. But maybe Go runtime can be improved on windows, and we can disable timeBeginPeriod call. Alex |
FWIW, an ETW trace (see https://github.com/google/UIforETW/releases for the easiest way to record one) can be used to record, among other things, all context switches. This makes it possible to determine where applications are waiting. The Windows timer resolution is a continuing source of frustration to be sure, but it only slows down code that calls Sleep(n) or WaitForSingleObject(handle, n) (where 'n' is not INFINITE). Looking at bench.txt I see that *OneShotTimeout-2 runs slower with a lower timer frequency. That sounds like it is by-design and shouldn't be used to justify all Go programs running with a high timer frequency. Ditto with BenchmarkGoroutineIdle-2. BenchmarkAppendGrowByte-2 runs 368% slower which suggests a benchmark bug. Either the code being tested is waiting (which it should not do) or the code that times is using timeGetTime and is stopping and starting the timer frequently and is therefore hitting aliasing errors. I strongly suspect that the latter is the problem. timeGetTime's precision is increased when the timer frequency is increased. The benchmarks with the lower timer precision are mostly not slower they are just less accurate. The correct fix is not to raise the timer frequency (the timers will still be inaccurate) but to use accurate timers such as QueryPerformanceCounter. It is a frustrating problem I'll agree, but raising the timer frequency so that benchmarks can measure the execution time better is not a great solution. If it turns out that the benchmarks such as BenchmarkAppendGrowByte-2 are actually slower when the timer frequency is lower then I will be surprised and might actually investigate. Fixing slow benchmarks requires understanding why they are slow. |
stepped on this issue searching for windows and time resolution issue for golang, not sure I see a workaround or solution though: Is there a better way to measure time spent than start := time.Now()
// ... do stuff...
elapsed := time.Since(start) which works fine on linux and macos but not on windows (~1ms resolution vs ns) |
There is no way to set ns time resolution on Windows. Alex |
Adding to what Alex said ... There is a way to get better resolution, but it's outside of what Go does for you. You can use the Windows Query Performance Counter function (Windows XP or later). It returns the duration since the program started. So you get a start and a stop time, then subtract to get the interval. Testing on my laptop showed about 550ns resolution with about 132ns call overhead. While 1us resolution isn't quite 1ns, it's better than 1ms. :) |
What does |
Correct. I posted https://go-review.googlesource.com/c/go/+/248699/ that fixes this issue. You get about 500ns resolution. Alex |
Sorry I was referring to smallest "waiting / sleeping" time. But if you just want to read time, time.Now is not the best source for "high res" time. You could call Windows APIs with better high res time. (There are so many I don't remember name) Alex |
@jstarks in case this is related... #35482 (comment) |
Thanks but how do I do that staying in golang? ps: I'm not a windows dev just trying to get my golang software to work on windows about the same as it runs on linux and mac, so ideally not making os dependant calls |
We should just fix this in Go rather than teach Windows developers to use Windows-specific code. Since Go uses CLOCK_MONOTONIC on Linux, and not CLOCK_MONOTONIC_COARSE, should we update Windows to use QueryUnbiasedInterruptTimePrecise/QueryInterruptTimePrecise to power time.Now()? |
See also #36141 - we need a new real-time timer API and to change the default policy on Windows as it differs from unix. |
This is how it is implemented go/src/runtime/sys_windows_amd64.s Line 499 in a386802
There is even version of that code that calls QPC. But version only runs on WINE.
I tried using ...Precise Windows APIs and they appear much slower on my tests. See #31160 (comment) for details. Alex |
This fix looks great! The test code was very handy. I modified it to check the current timer resolution before running (since otherwise the results may not be completely valid) and made some other changes and checked it in here: https://github.com/randomascii/blogstuff/blob/main/timer_interval/waitable_timer.cpp I verified your results with the timer check present.. @jstarks - any progress on getting this documented? I just checked in CreateWaitableTimerEx still doesn't mention this. The hader makes it look like this is available in 1803 and above but it would be great to confirm this. Chromium might be interested in this. |
Change https://golang.org/cl/271907 mentions this issue: |
This change adjusts usleep2HighRes so it does not crash when TLS is not configured. When g is not available, usleep2HighRes just calls usleep2 instead. Updates #8687 Change-Id: Idbb80f7b71d1da350a6a7df7c49154eb1ffe29a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/271907 Run-TryBot: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Simon Rozman <simon@rozman.si> Trust: Jason A. Donenfeld <Jason@zx2c4.com> Trust: Alex Brainman <alex.brainman@gmail.com>
We carried a custom implementation of timeutil.Now() for Windows, to overcome the low precision of Go's impl [1]. It was doing so with an expensive implementation. Having this custom implementation also make it tricky to use stdlib functions like Since(), which internally use the stdlib Now(). The Golang issue has since been fixed. [1] golang/go#8687 Release note:
We carried a custom implementation of timeutil.Now() for Windows, to overcome the low precision of Go's impl [1]. It was doing so with an expensive implementation. Having this custom implementation also make it tricky to use stdlib functions like Since(), which internally use the stdlib Now(). The Golang issue has since been fixed. [1] golang/go#8687 Release note: None
We carried a custom implementation of timeutil.Now() for Windows, to overcome the low precision of Go's impl [1]. It was doing so with an expensive implementation. Having this custom implementation also make it tricky to use stdlib functions like Since(), which internally use the stdlib Now(). The Golang issue has since been fixed. [1] golang/go#8687 Release note: None
We carried a custom implementation of timeutil.Now() for Windows, to overcome the low precision of Go's impl [1]. It was doing so with an expensive implementation. Having this custom implementation also make it tricky to use stdlib functions like Since(), which internally use the stdlib Now(). The Golang issue has since been fixed. [1] golang/go#8687 Release note: None
We carried a custom implementation of timeutil.Now() for Windows, to overcome the low precision of Go's implementation [1]. Our implementation was expensive, though. Having this custom implementation also makes it tricky to use stdlib functions like Since(), which internally use the stdlib Now(). The Golang issue has since been fixed, so our custom implementation can go away. [1] golang/go#8687 Release note: None
The text was updated successfully, but these errors were encountered: