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/debug: FreeOSMemory() not working on Android 9 #37569

Closed
YouROK opened this issue Feb 28, 2020 · 10 comments
Closed

runtime/debug: FreeOSMemory() not working on Android 9 #37569

YouROK opened this issue Feb 28, 2020 · 10 comments

Comments

@YouROK
Copy link

@YouROK YouROK commented Feb 28, 2020

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

GO 1.12 - 1.14

Does this issue reproduce with the latest release?

Yes

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

On android 9, on other all works

What did you do?

func main() {
	bigArr := make([]byte, 250*1024*1024)
	for i := 0; i < len(bigArr); i++ {
		bigArr[i] = byte(i)
	}
	bigArr = nil
	runtime.GC()
	debug.FreeOSMemory()
	for true {
		//Some thing works in program
		time.Sleep(time.Second)
	}
}

What did you expect to see?

I expected to see the memory in the system, but it remains in the program. If you allocate memory several times, then the program is killed by the system, but should clear it

In version 1.11.X all works in android 9, memory returning to os, but not execute on android 10. On android 5,6,7,8 all works of new golang version

Compile cmd: GOARM=7 GOOS=linux GOARCH=arm go build -o test main

@odeke-em odeke-em changed the title debug.FreeOSMemory() not working on android 9 runtime/debug: FreeOSMemory() not working on Android 9 Mar 1, 2020
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Mar 1, 2020

Thank you for the report @YouROK and welcome to the Go project!

/cc @mknyszek @aclements @eliasnaur @cherrymui

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Jul 1, 2020

Considering the combination of Android version and Go version here (Android 10+, Go 1.12+), I suspect this is another case of MADV_FREE not updating the kernel's per-process RSS numbers. Android 10's minimum Linux kernel version is 4.9 whereas Android 9's minimum is 4.4. MADV_FREE was introduced in Linux 4.5.

Sorry for the very late reply on this. Could you try running your code with GODEBUG=madvdontneed=1? I'm not sure how difficult that is to do on Android.

@elgatito
Copy link

@elgatito elgatito commented Aug 18, 2020

@mknyszek It is great we can use GODEBUG flags from environment variables. But it would be alto great to be able to hardcode that into the code, or define, depending on the build parameters, which is needed, if you cannot control how the binary is ran by user.

@networkimprov
Copy link

@networkimprov networkimprov commented Aug 18, 2020

Filed #40870

@aclements
Copy link
Member

@aclements aclements commented Sep 4, 2020

It's really disappointing if Android kills applications for memory use without first trying to clean up their freed pages... If that is the case, then I would consider MADV_FREE quite broken on Android and we shouldn't use it.

@YouROK, your example code is likely working as expected, but you mention "If you allocate memory several times, then the program is killed by the system". That sounds like the real issue here. Could you post code that reproduces that?

@aclements
Copy link
Member

@aclements aclements commented Nov 1, 2020

Ping @YouROK . Could you try the GODEBUG=madvdontneed=1 environment variable? And can you post the code that demonstrates "allocating memory several times"?

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 2, 2020

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

gopherbot pushed a commit that referenced this issue Nov 2, 2020
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 under OS memory pressure. 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.

This CL changes 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. This can still be overridden by
setting GODEBUG=madvdontneed=0.

Fixes #42330 (meta-issue).

Fixes #41818, #39295, #37585, #33376, #30904 (many of which were
already closed as "working as intended").

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

@gopherbot gopherbot commented Dec 1, 2020

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this Dec 1, 2020
@odeke-em odeke-em removed the WaitingForInfo label Dec 2, 2020
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 2, 2020

Re-opening this issue.

@odeke-em odeke-em reopened this Dec 2, 2020
@aclements
Copy link
Member

@aclements aclements commented Dec 2, 2020

I think gopherbot actually did the right thing here. We haven't heard back from @YouROK, so there's not anything more we can do here, and whatever the details of this issue are, it's probably fixed by CL 267100. So I'm going to re-close this issue, but feel free to reopen it with more information, @YouROK .

@aclements aclements closed this Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.