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

runtime: Interferes with Windows timer frequency #28255

Open
stevenh opened this issue Oct 17, 2018 · 19 comments
Open

runtime: Interferes with Windows timer frequency #28255

stevenh opened this issue Oct 17, 2018 · 19 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@stevenh
Copy link
Contributor

stevenh commented Oct 17, 2018

What version of Go are you using (go version)?

go version go1.11.1 windows/amd64

Does this issue reproduce with the latest release?

Try to set a high frequency timer

What operating system and processor architecture are you using (go env)?

windows amd64

What did you do?

Set the OS to a high frequency tick using NtSetTimerResolution

What did you expect to see?

NtQueryTimerResolution should return 1ms

What did you see instead?

NtQueryTimerResolution returned 15ms

This used to work but was broken by the following commit:
11eaf42

This "reduces the timer resolution when the Go process is entirely idle" however this is a system wide setting so golang cannot just assume another app on the machine doesn't require a high resolution timer to work correctly, hence setting it back to 15ms isn't safe.

stevenh referenced this issue Oct 17, 2018
Currently Go sets the system-wide timer resolution to 1ms the whole
time it's running. This has negative affects on system performance and
power consumption. Unfortunately, simply reducing the timer resolution
to the default 15ms interferes with several sleeps in the runtime
itself, including sysmon's ability to interrupt goroutines.

This commit takes a hybrid approach: it only reduces the timer
resolution when the Go process is entirely idle. When the process is
idle, nothing needs a high resolution timer. When the process is
non-idle, it's already consuming CPU so it doesn't really matter if
the OS also takes timer interrupts more frequently.

Updates #8687.

Change-Id: I0652564b4a36d61a80e045040094a39c19da3b06
Reviewed-on: https://go-review.googlesource.com/38403
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
@stevenh
Copy link
Contributor Author

stevenh commented Oct 17, 2018

The following code demonstrates the issue:

package main

import (
	"log"
	"syscall"
	"time"
	"unsafe"
)

var (
	ntdll = syscall.NewLazyDLL("ntdll.dll")

	procNtQueryTimerResolution = ntdll.NewProc("NtQueryTimerResolution")
	procNtSetTimerResolution   = ntdll.NewProc("NtSetTimerResolution")
)

// QueryTimerResolution gets the min, max and current timer resolution.
// minRes is highest possible delay (in 100-ns units) between timer events.
// maxRes is lowest possible delay (in 100-ns units) between timer events.
// curRes is the current timer resolution, in 100-ns units.
func QueryTimerResolution() (minRes, maxRes, curRes uint32, err error) {
	ret, _, err := procNtQueryTimerResolution.Call(
		uintptr(unsafe.Pointer(&minRes)),
		uintptr(unsafe.Pointer(&maxRes)),
		uintptr(unsafe.Pointer(&curRes)),
	)

	if ret == 0 {
		err = nil
	}

	return
}

// SetTimerResolution sets the timer resolution (to the nearest allowed value).
// curRes is the actual timer resolution that was set, in 100-ns units.
func SetTimerResolution(newRes uint32) (curRes uint32, err error) {
	var setRes uint32

	setRes = 1
	ret, _, err := procNtSetTimerResolution.Call(
		uintptr(newRes),
		uintptr(setRes),
		uintptr(unsafe.Pointer(&curRes)),
	)

	if ret == 0 {
		err = nil
	}

	return
}

func main() {
	if min, max, cur, err := QueryTimerResolution(); err != nil {
		log.Fatal(err)
	} else {
		log.Println("min:", min, ", max:", max, ", cur:", cur)
	}

	if cur, err := SetTimerResolution(1000); err != nil {
		log.Fatal(err)
	} else {
		log.Println("cur:", cur)
	}

	for range time.Tick(time.Second * 10) {
		if min, max, cur, err := QueryTimerResolution(); err != nil {
			log.Fatal(err)
		} else {
			log.Println("min:", min, ", max:", max, ", cur:", cur)
		}
	}
}

If you run this code on a Windows host with a default timer resolution of 15.6ms and a min resolution of 0.5ms you should get a current resolution of 0.5ms however with osRelax in play you will see a mixture of 0.5ms and 15.6ms.

If you disable osRelax e.g. return 0 always then you will get the correct behaviour of always 0.5ms for example:
osRelax Enabled

2018/10/17 21:05:51 min: 156250 , max: 5000 , cur: 9765
2018/10/17 21:05:51 cur: 4882
2018/10/17 21:05:52 min: 156250 , max: 5000 , cur: 156250
2018/10/17 21:05:53 min: 156250 , max: 5000 , cur: 156250
2018/10/17 21:05:54 min: 156250 , max: 5000 , cur: 156250
2018/10/17 21:05:55 min: 156250 , max: 5000 , cur: 156250
2018/10/17 21:05:56 min: 156250 , max: 5000 , cur: 156250

osRelax Disabled

2018/10/17 21:08:12 min: 156250 , max: 5000 , cur: 156250
2018/10/17 21:08:12 cur: 4882
2018/10/17 21:08:13 min: 156250 , max: 5000 , cur: 4882
2018/10/17 21:08:14 min: 156250 , max: 5000 , cur: 4882
2018/10/17 21:08:15 min: 156250 , max: 5000 , cur: 4882
2018/10/17 21:08:16 min: 156250 , max: 5000 , cur: 4882
2018/10/17 21:08:17 min: 156250 , max: 5000 , cur: 4882

@aclements
Copy link
Member

Oh, now I understand better what you meant in your comment on that commit.

The problem isn't that the timer resolution is a system-wide setting. What Go does with the timer resolution won't affect other processes on the system that want a high-resolution timer. The problem is that the user Go code wants a high-resolution timer, and this is a per-process setting.

Anything that's per-process like this is going to be problematic (POSIX signals are another great example of this, which is why we abstract them away in Go). There's nothing we can do if the program reaches directly for Windows APIs, especially undocumented ones like NtSetTimerResolution. We could potentially expose TimeBeginPeriod and TimeEndPeriod from the syscall package and make those cooperate with the runtime, though then we're just inflicting the badness of this API on users.

Ideally we'd make the runtime itself not depend on high-resolution timers. I'm not entirely sure what that would take, and even then this gets in the way of portability because people expect to be able to write Go code that, say, sleeps for 1 millisecond, and then that code doesn't work as expected on Windows.

cc @alexbrainman for any insights.

@stevenh
Copy link
Contributor Author

stevenh commented Oct 17, 2018

I've been doing some more digging and it turns out that a second call to timeBeginPeriod(1) within the same go process actually allows NtSetTimerResolution to operate correctly.

What is even more odd is there is a strange interaction between the two functions:

  1. timeBeginPeriod(1) + NtSetTimerResolution(5000) results in a timer resolution of 0.5ms
  2. timeBeginPeriod(2) + NtSetTimerResolution(5000) results in a timer resolution of 2ms
  3. timeBeginPeriod(1) on its own results in a timer resolution of 1ms

Just to be clear both these functions have a system global effect not a per process one:
https://docs.microsoft.com/en-us/windows/desktop/api/timeapi/nf-timeapi-timebeginperiod

This function affects a global Windows setting. Windows uses the lowest value (that is, highest resolution) requested by any process.

The strange thing is there is an interaction between timeBeginPeriod / timeEndPeriod and NtSetTimerResolution if they are both called from within the same process.

@alexbrainman
Copy link
Member

What did you expect to see?

NtQueryTimerResolution should return 1ms

What did you see instead?

NtQueryTimerResolution returned 15ms

Your goal is to have highres timer. But some people would like their laptop battery last longer (#8687). Why should Go runtime side with your goal? 11eaf42 is trying to please both sides - when runtime has work to do, highres timer is on, when runtime is idle highres timer is off. I think 11eaf42 is doing best job possible in this situation. If you need something different, you should be able to adjust runtime as you pleased.

cc @alexbrainman for any insights.

I am out of insights in this matter long time ago. :-) Sorry.

Alex

@stevenh
Copy link
Contributor Author

stevenh commented Oct 18, 2018

Both are good goals, so lets see if we can satisfy both, I see two possible options:

  1. Clearly document the issue and provide the workaround; manually call timeBeginPeriod.
  2. Have osRelax detect if an external agent has requested a high resolution timer and use that knowledge to ensure it doesn't interfere with that.

For the latter osRelax(false) could check the frequency already in use using NtGetTimerResolution and only call timeBeginPeriod if the frequency is not already high enough. Then on for osRelax(true) only call timeEndPeriod if it had made the corresponding call to timeBeginPeriod.

@alexbrainman
Copy link
Member

runtime package actually needs timeBeginPeriod itself. How do you propose handle that?

Alex

@stevenh
Copy link
Contributor Author

stevenh commented Oct 18, 2018

From what I've seen there is only a single use, that is to increase the timer resolution to ensure that the runtime operates well. If the required timer resolution is already in place there is no need to make this request, so this shouldn't have any impact on the runtime.

Here's the sort of thing I have in mind:

var timeBeginCalls int

// highResTimer returns true if the current timer resolution is know to be 1ms or better, false otherwise.
func highResTimer() bool {
    if timeBeingCalls > 0 {
         return true
    }
    var minRes, maxRes, curRes uint32
    ret := stdcall3(_NtQueryTimerResolution,
           uintptr(unsafe.Pointer(&minRes)),
           uintptr(unsafe.Pointer(&maxRes)),
           uintptr(unsafe.Pointer(&curRes)),
     )
     if ret == 0 {
           return curRes <= 10000
     }
     return false
}

func osRelax(relax bool) uint32 {
        if relax {
                if timeBeginCalls > 0 {
                        timeBeginCalls--
                        return uint32(stdcall1(_timeEndPeriod, 1))
                }
        } else if !highResTimer() {
                timeBeginCalls++
                return uint32(stdcall1(_timeBeginPeriod, 1))
        }

        return 0
}

@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 19, 2018
@FiloSottile FiloSottile added this to the Unplanned milestone Oct 19, 2018
@alexbrainman
Copy link
Member

Here's the sort of thing I have in mind:

@stevenh thank you for explaining. You suggest we use undocumented NtQueryTimerResolution. How do you propose we make sure that your change does actually work. And not just on your computer and mine?

And thinking about your original issue. Why cannot you just call timeBeginPeriod ? From your Go program or from any program on your computer. That should have effect you want. No?

Alex

@stevenh
Copy link
Contributor Author

stevenh commented Oct 24, 2018

That does indeed workaround the issue, but the problem remains that the go runtime is making changes to the system that impact how users code works, without either documenting it or providing the option to prevent it from doing so.

The underlying issue that timeBeingPeriod and NtQueryTimerResolution don't play nice and that timeBeginPeriod isn't a exact replacement for NtQueryTimerResolution i.e. it lacks the ability to set sub 1ms frequency.

We have the workaround we need, but the question remaining is how do we prevent others from being bitten by the same issue as very hard to spot?

@aclements
Copy link
Member

For the latter osRelax(false) could check the frequency already in use using NtGetTimerResolution and only call timeBeginPeriod if the frequency is not already high enough. Then on for osRelax(true) only call timeEndPeriod if it had made the corresponding call to timeBeginPeriod.

The problem with this is that, if user Go code is running and able to call NtSetTimerResolution, the runtime has already adjusted the timer frequency. The runtime will never see an adjusted timer resolution when calling osRelax(false).

You suggest we use undocumented NtQueryTimerResolution. How do you propose we make sure that your change does actually work. And not just on your computer and mine?

@alexbrainman, does Microsoft have a policy on undocumented APIs like this? My impression is that some are in pretty wide-spread use, which would seem to make them de facto standard.

Specifically, I'm wondering if we can provide a more abstract API from syscall that uses NtSetTimerResolution if it can or TimeBeginPeriod otherwise and interacts with the runtime's use appropriately.

@stevenh
Copy link
Contributor Author

stevenh commented Oct 24, 2018

While NtSetTimerResolution is undocumented its used a lot in the wild so, as you say is kind of a defacto standard. I've not found a Windows version yet which doesn't provide it, not to say that won't change in the future though.

While I can't prove it, it seems likely that timeBeginPeriod and timeEndPeriod are actually built on top NtSetTimerResolution with per process reference accounting. I expect that NtSetTimerResolution is called with false when the reference hits zero on timeEndPeriod which would explain the behaviour we're seeing.

With regards to providing an syscall the problem with falling back to timeBeginPeriod is they aren't functionally equivalent when it comes to the maximum frequencies they support.

@aclements
Copy link
Member

While I can't prove it, it seems likely that timeBeginPeriod and timeEndPeriod are actually built on top NtSetTimerResolution with per process reference accounting.

Surely NtSetTimerResolution also sets a per-process setting like Time{Begin,End}Period, not a system-wide setting? It would seem completely unusable if it were system-wide. If two processes use NtSetTimerResolution to set high resolution, and then one of them lowers its resolution, presumably the system continues to run at high resolution?

With regards to providing an syscall the problem with falling back to timeBeginPeriod is they aren't functionally equivalent when it comes to the maximum frequencies they support.

I understand that, but if NtSetTimerResolution isn't available and TimeBeginPeriod is the only API provided by the kernel, there's not much we can do but use TimeBeginPeriod.

@stevenh
Copy link
Contributor Author

stevenh commented Oct 25, 2018

While I can't prove it, it seems likely that timeBeginPeriod and timeEndPeriod are actually built on top NtSetTimerResolution with per process reference accounting.

Surely NtSetTimerResolution also sets a per-process setting like Time{Begin,End}Period, not a system-wide setting? It would seem completely unusable if it were system-wide. If two processes use NtSetTimerResolution to set high resolution, and then one of them lowers its resolution, presumably the system continues to run at high resolution?

They are both a system-wide setting, based on the highest resolution set by a process.

NtSetTimerResolution can be called to set timer resolutions by more than on application. To support a subsequent process setting a timer resolution without violating the resolution assumptions of a previous caller, NtSetTimerResolution never lowers the timer's resolution, only raises it. For example, if a process sets the resolution to 5 milliseconds, subsequent calls to set the resolution to between 5 and 10 millseconds will return a status code indicating success, but the timer will be left at 5 milliseconds.

NtSetTimerResolution also keeps track of whether a process has set the timer resolution in its process control block, so that when a call is made with Set equal to FALSE it can verify that the caller has previously requested a new resolution. Every time a new resolution is set a global counter is incremented, and every time it is reset the counter is decremented. When the counter becomes 0 on a reset call the timer is changed back to its default rate, otherwise no action is taken. Again, this preserves the timer resolution assumptions of all the applications that have requested high resolution timers by guaranteeing that the resolution will be at least as good as what they specified.
Source: http://mirrors.arcadecontrols.com/www.sysinternals.com/Information/HighResolutionTimers.html

With regards to providing an syscall the problem with falling back to timeBeginPeriod is they aren't functionally equivalent when it comes to the maximum frequencies they support.

I understand that, but if NtSetTimerResolution isn't available and TimeBeginPeriod is the only API provided by the kernel, there's not much we can do but use TimeBeginPeriod.

Indeed its more around making it very clear to the user.

@alexbrainman
Copy link
Member

While NtSetTimerResolution is undocumented its used a lot in the wild so, as you say is kind of a defacto standard. I've not found a Windows version yet which doesn't provide it, not to say that won't change in the future though.

I agree. Microsoft will try to support broken programs that use API incorrectly or use undocumented API.

Alex

@randomascii
Copy link

Any chance this will get addressed? It would be nice to at least have some easy way for specific Go programs to disable the dynamic raising of the timer interrupt frequency.

I work at Google and my workstation has several custom Go programs running on it at all times. This means that the timer interrupt frequency is essentially always raised. This means that it is impossible for me to test how the Chrome web browser behaves on a system running with the normal timer interrupt frequency. This is tracked by the Google internal bug b/133354039 and by the public Chromium bug crbug.com/927165.

people expect to be able to write Go code that, say, sleeps for 1 millisecond,
and then that code doesn't work as expected on Windows.

I would argue that on Windows that is not actually a reasonable expectation. Go needs to be portable, but it also needs to respect platform norms and expectations. Probably the only way to resolve these conflicting desires is to provide an API or environment variable to let programs specify whether they want the automatic raising of the timer interrupt frequency. I think it should default to off, but the default value isn't as important as creating such a method. Only then can my coworkers use that API to make the Go programs they put on my machine not raise the timer interrupt frequency, and then I can try to make Chromium more power efficient.

@ianlancetaylor
Copy link
Contributor

Is there any difference between this issue and #8687? #8687 is marked as a release blocker for the 1.15 release. Should we close this issue as a dup of that one?

@the80srobot
Copy link

I agree with randomascii that expecting fine-grained timers where not supported by the OS is the exceptional case.

I might be wrong, but doesn't macOS force you to accept coarse timers when it decides it needs to save power? Wake-ups get coalesced in the kernel AFAIK. Go seems to work just fine on that platform - why is Windows special? Is it just that we have a lever to pull, so we pull it?

@aclements
Copy link
Member

@ianlancetaylor, they're certainly related. I think this issue has become more about providing an API to give users control over the timer resolution, while #8687 is about fixing the runtime to not require high timer resolution. It may be that we need to solve both together, though, so perhaps the issues should be folded together.

@networkimprov
Copy link

@randomascii recently blogged this change of timer behavior to Windows 10 in mid-2020:
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
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

10 participants