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.

Sign up for GitHub

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

Jump to bottom

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

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

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

YouROK opened this issue Feb 28, 2020 · 13 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Android

Comments

@YouROK
Copy link

YouROK commented Feb 28, 2020 •
edited by odeke-em
Loading

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 commented Mar 1, 2020

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

/cc @mknyszek @aclements @eliasnaur @cherrymui

@odeke-em odeke-em added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2020
@mknyszek
Copy link
Contributor

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

@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 networkimprov mentioned this issue Aug 18, 2020
Closed
@networkimprov
Copy link

Filed #40870

@aclements
Copy link
Member

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 aclements added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 1, 2020
@aclements
Copy link
Member

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

@aclements aclements mentioned this issue Nov 2, 2020
Closed
@gopherbot
Copy link
Contributor

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
@aclements
runtime: default to MADV_DONTNEED on Linux
05e6d28 
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>
@YouROK YouROK mentioned this issue Nov 11, 2020
Closed
@gopherbot
Copy link
Contributor

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.)

@odeke-em odeke-em removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 2, 2020
@odeke-em
Copy link
Member

odeke-em commented Dec 2, 2020

Re-opening this issue.

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

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 .

@YouROK
Copy link
Author

YouROK commented Mar 3, 2021 •
edited
Loading

@aclements with flag GODEBUG=madvdontneed=1 all work good.
I have been looking for the problem on different devices for several months.
I can say for sure that the problem exists on almost all Linux devices, but the memory grows only up to a certain point, which is not critical on a regular PC with Linux. But on android, Linux and Android builds behave the same way, and when a certain point is reached, the Android OS closes the application.
I can suggest one solution to enable the build flag with GODEBUG=madvdontneed=1 to not add it at startup in env
Last test be in go 1.16

@networkimprov
Copy link

networkimprov commented Mar 3, 2021 •
edited
Loading

@gopherbot add OS-Android

EDIT: @odeke-em could you tag with os-android? Gopherbot can't label closed issues...

@aclements
Copy link
Member

@YouROK, Go 1.16 defaults to GODEBUG=madvdontneed=1, so setting that environment in Go 1.16 should not have any effect.

@golang golang locked and limited conversation to collaborators Jul 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Android
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@networkimprov @elgatito @mknyszek @aclements @YouROK @odeke-em @gopherbot