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: default to MADV_DONTNEED on Linux #42330

Closed
aclements opened this issue Nov 2, 2020 · 10 comments
Closed

runtime: default to MADV_DONTNEED on Linux #42330

aclements opened this issue Nov 2, 2020 · 10 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@aclements
Copy link
Member

In Go 1.12, we changed the runtime to use MADV_FREE when available on Linux (falling back to MADV_DONTNEED) in CL 135395 to address issue #23687. While MADV_FREE is somewhat faster than MADV_DONTNEED, it doesn't affect many of the statistics that MADV_DONTNEED does until the memory is actually reclaimed. This generally leads to poor user experience, like confusing stats in top and other monitoring tools; and bad integration with management systems that respond to memory usage.

We've seen numerous issues about this user experience, including #41818, #39295, #37585, #33376, and #30904, many questions on Go mailing lists, and requests for mechanisms to change this behavior at run-time, such as #40870. There are also issues that may be a result of this, but root-causing it can be difficult, such as #41444 and #39174. And there's some evidence it may even be incompatible with Android's process management in #37569.

I propose we change the default to prefer MADV_DONTNEED over MADV_FREE, to favor user-friendliness and minimal surprise over performance. I think it's become clear that Linux's implementation of MADV_FREE ultimately doesn't meet our needs. We've also made many improvements to the scavenger since Go 1.12. In particular, it is now far more prompt and it is self-paced, so it will simply trickle memory back to the system a little more slowly with this change.

/cc @mknyszek @rsc

@aclements aclements added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 2, 2020
@aclements aclements added this to the Go1.16 milestone Nov 2, 2020
@gopherbot
Copy link

Change https://golang.org/cl/267100 mentions this issue: runtime: default to MADV_DONTNEED on Linux

@richardartoul
Copy link

Just gonna throw my 2c in that I write a lot of performance critical software in Go and I set them all to use MADV_DONTNEED because otherwise monitoring memory usage becomes a nightmare so I would fully support this proposal. I think MADV_DONTNEED is the more sane default and if someone has some very specific and sensitive workload then they can switch to MADV_FREE

@mknyszek
Copy link
Contributor

mknyszek commented Nov 2, 2020

MADV_DONTNEED generally performs worse (in a vacuum, 1.5-2x, likely worse in practice due to differences in locking in the kernel), but switching to that shouldn't affect the performance of Go code since the scavenger paces itself to limit its CPU time regardless of how expensive it is to scavenge. The one thing it will affect is how quickly an application can recover from a heap spike, which today is mostly governed by how many free huge pages are available to the scavenger anyway.

I think the monitoring problem is much more severe, and so I support changing the default on Linux. I've responded to a number of these issues and I don't see an end to them. MADV_FREE is just plain confusing.

Furthermore, I don't really even see a use in supporting MADV_FREE as an option on Linux at all. Other allocators such as TCMalloc don't support MADV_FREE, despite memory allocators being the target use-case. Last I checked jemalloc supports it, but in a two-tiered approach: first MADV_FREE, then MADV_DONTNEED. This can get kind of hairy and complicated and I'm not sure if there's a lot of benefit to doing it this way for us.

@TwiN
Copy link

TwiN commented Nov 2, 2020

Definitely agree with this proposal, as before I was even aware of the existence of MADV_DONTNEED and MADV_FREE, it was quite troublesome to find out why this behavior happened in the first place.

That said, I am a bit curious about the performance implications between the two; I might run some benchmarks later.

@mknyszek
Copy link
Contributor

mknyszek commented Nov 2, 2020

@TwinProduction Feel free to try out benchmarks, but unless your benchmark involves periodic heap spikes (and even then, I'd only expect MemStats.HeapSys-MemStats.HeapReleased to actually change) I wouldn't be surprised if you don't see any difference. The runtime actively tries to smooth out performance differences in different implementations of MADV_DONTNEED/MADV_FREE/MEM_DECOMMIT/etc. across platforms.

@gopherbot
Copy link

Change https://golang.org/cl/267137 mentions this issue: doc/go1.16: document switch to MADV_DONTNEED

gopherbot pushed a commit that referenced this issue Nov 2, 2020
Updates #42330.

Change-Id: Ifda10a5c3dca30acf1258e9e0af202e9beffc68e
Reviewed-on: https://go-review.googlesource.com/c/go/+/267137
Trust: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
@franklwel1990
Copy link

MADV_FREE会导致很多问题的存在, 一台主机往往不可能只放一个go应用程序跑,还会跑其它很多应用。
当我执行了一次go请示并且申请了很大块系统内存占用着一直不释放归还操作系统,简直不可思议,不仅导致其它应用程序无法运行和报错。
使用这种MADV_FREE模式每次都是内存被消耗完go进程被killed,完全没有释放的可能和降低的机制。
系统指标无法评测

@fanlv
Copy link

fanlv commented Nov 9, 2020

MADV_FREE会导致很多问题的存在, 一台主机往往不可能只放一个go应用程序跑,还会跑其它很多应用。
当我执行了一次go请示并且申请了很大块系统内存占用着一直不释放归还操作系统,简直不可思议,不仅导致其它应用程序无法运行和报错。
使用这种MADV_FREE模式每次都是内存被消耗完go进程被killed,完全没有释放的可能和降低的机制。
系统指标无法评测

你可以 GODEBUG=madvdontneed=1强制使用_MADV_DONTNEED就行了。

@aclements
Copy link
Member Author

aclements commented Nov 9, 2020

@franklwel1990 , I'm reading an automatic translation of your message, so forgive me if I misunderstand anything. Go has used MADV_FREE since Go 1.12. It should not cause other applications to fail, but you're right that it can make the system hard to measure. Go 1.16 will use MADV_DONTNEED.

@vtolstov
Copy link

How about to backport it to go 1.15?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

8 participants