-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: scavenger freezes up in Go 1.14 in Windows due to coarse time granularity #38617
Comments
Thanks for the issue. This is expected on linux and RSS will drop if the system is under memory pressure. But this is Windows. Is the same behavior expected @aclements @mknyszek ? |
On Windows it should be eager. Firstly I'll note that what the reproducer is printing doesn't subtract out HeapReleased from HeapIdle, so it's not going to give an accurate picture of what the runtime thinks it returned to the OS. The fact that Windows' Task Manager is reporting that high of an RSS is concerning, considering that Windows reports physical memory used. I ran this on a Windows VM and Puzzled, I started printing some diagnostics, and noticed that scavenge pacing math was producing a bunch of So, this is a real bug. I'm not sure what the fix is, and it may be worth back-porting to 1.14? There is a workaround, which is to call |
As @aclements informed me it looks like the theoretical granularity should be 100ns, which would be enough, but the time is only updated on every OS tick. Experimentally, I found in my Windows VM that the best granularity I could get (spin until nanotime()-start != 0 in the scavenger) was around 0.95ms, which is definitely not good enough. I think we need a fallback here to the Go 1.13 pacing which didn't rely on measuring time. CC @aclements |
@aclements also just informed me that Go sets Windows' interrupt frequency to 1ms, so there we go. |
Change https://golang.org/cl/229997 mentions this issue: |
I've confirmed that the change which closed this issue fixed the scavenger not kicking in. However, because of a lack of feedback, the Go runtime has to be conservative, which means about a 3-4 MiB/s release rate back to the OS. Perhaps we should think about being more aggressive about scavenging when the application goes idle, though it's hard because we don't know when the application will wake back up again to do work. |
@gopherbot please backport to 1.14; this is a regression. |
Backport issue(s) opened: #38856 (for 1.14). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Change https://golang.org/cl/232743 mentions this issue: |
…re defensive This change adds two bits of logic to the scavenger's pacing. Firstly, it checks to make sure we scavenged at least one physical page, if we released a non-zero amount of memory. If we try to release less than one physical page, most systems will release the whole page, which could lead to memory corruption down the road, and this is a signal we're in this situation. Secondly, the scavenger's pacing logic now checks to see if the time a scavenging operation takes is measured to be exactly zero or negative. The exact zero case can happen if time update granularity is too large to effectively capture the time the scavenging operation took, like on Windows where the OS timer frequency is generally 1ms. The negative case should not happen, but we're being defensive (against kernel bugs, bugs in the runtime, etc.). If either of these cases happen, we fall back to Go 1.13 behavior: assume the scavenge operation took around 10µs per physical page. We ignore huge pages in this case because we're in unknown territory, so we choose to be conservative about pacing (huge pages could only increase the rate of scavenging). Currently, the scavenger is broken on Windows because the granularity of time measurement is around 1 ms, which is too coarse to measure how fast we're scavenging, so we often end up with a scavenging time of zero, followed by NaNs and garbage values in the pacing logic, which usually leads to the scavenger sleeping forever. For #38617. Fixes #38856. Change-Id: Iaaa2a4cbb21338e1258d010f7362ed58b7db1af7 Reviewed-on: https://go-review.googlesource.com/c/go/+/229997 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Austin Clements <austin@google.com> (cherry picked from commit c791537) Reviewed-on: https://go-review.googlesource.com/c/go/+/232743
This change adds two bits of logic to the scavenger's pacing. Firstly, it checks to make sure we scavenged at least one physical page, if we released a non-zero amount of memory. If we try to release less than one physical page, most systems will release the whole page, which could lead to memory corruption down the road, and this is a signal we're in this situation. Secondly, the scavenger's pacing logic now checks to see if the time a scavenging operation takes is measured to be exactly zero or negative. The exact zero case can happen if time update granularity is too large to effectively capture the time the scavenging operation took, like on Windows where the OS timer frequency is generally 1ms. The negative case should not happen, but we're being defensive (against kernel bugs, bugs in the runtime, etc.). If either of these cases happen, we fall back to Go 1.13 behavior: assume the scavenge operation took around 10µs per physical page. We ignore huge pages in this case because we're in unknown territory, so we choose to be conservative about pacing (huge pages could only increase the rate of scavenging). Currently, the scavenger is broken on Windows because the granularity of time measurement is around 1 ms, which is too coarse to measure how fast we're scavenging, so we often end up with a scavenging time of zero, followed by NaNs and garbage values in the pacing logic, which usually leads to the scavenger sleeping forever. Fixes golang#38617. Change-Id: Iaaa2a4cbb21338e1258d010f7362ed58b7db1af7 Reviewed-on: https://go-review.googlesource.com/c/go/+/229997 Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Austin Clements <austin@google.com>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Used memory cannot be freed.
The text was updated successfully, but these errors were encountered: