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: shortage of contiguous virtual memory on Android L #9311

Closed
crawshaw opened this issue Dec 14, 2014 · 12 comments
Closed

runtime: shortage of contiguous virtual memory on Android L #9311

crawshaw opened this issue Dec 14, 2014 · 12 comments

Comments

@crawshaw
Copy link
Member

On 32-bit ARM, Go initially reserves a single block of 768 MB of virtual memory (runtime/malloc1.go:146). According to several reports, this fails in some L apps. It appears that the ART runtime in the same process reserves more than twice the -Xmx heap size in virtual memory.

A user inside Google with a working reproduction was about to modify the runtime to allocate an initial 512 MB block. This means we could switch to the alternate reservation scheme described in the malloc1.go comments and safely use smaller arena sizes.

The minimal change is to shrink the initial arena size to 256 MB. This seems reasonable for the Android apps I know of, but I am not sure how it would affect other 32-bit users.

@crawshaw
Copy link
Member Author

I'm experimenting with the minimal change, https://go-review.googlesource.com/1514, but I don't have a device that reproduces the bug yet.

@minux
Copy link
Member

minux commented Dec 14, 2014

For the 32-bit ports, probably we should switch to a scheme that does not
rely on
a single bitmap for all the Go heap.

A similar problem exists on windows/386, that some DLL loads in the middle
of
the user's address space, and fragmented the VM space so that there is
enough
VM in total, but there is no 768MB of contiguous VM space. On windows this
problem is exaggerated by the fact that by default, each user process only
has
2GB of VM space.

Do we know why the ART runtime reserves so much VM space?

Could we make the change such that it first tries 512MB arena, and if
failed,
print a message to logcat (perhaps more importantly, cat a copy of
/proc/self/maps
to logcat to aid diagnosis, could sandboxed app read /proc/self/maps?) and
fallback to 256MB arena?

@minux
Copy link
Member

minux commented Dec 14, 2014

I filed #9313 for the multiple bitmaps enhancement request.

@crawshaw
Copy link
Member Author

The ART team believe they can reduce the VM use, but are not sure when they can do it. I think fixing this is on us.

@randall77
Copy link
Contributor

It seems reasonable to try a smaller arena if the large arena allocation fails.
However, it seems that whomever gets to allocate their heap first (Go or ART) gets the bulk of the address space and the other gets the dregs. I don't see any obvious way of being fairer, as when one is being allocated it doesn't know about the future allocation by the other.

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014
@jhleath
Copy link

jhleath commented Dec 19, 2014

Even after this commit, I get crashes that are very similar to what I was experiencing beforehand (only now some amount of Go code is executed). Still nothing informative in logcat (unless I need to do something than the standard adb logcat to get more information).

This wouldn't be related if the original error had to do with arena allocation since the arena is only allocated once at the beginning of the program, correct?

@minux
Copy link
Member

minux commented Dec 19, 2014

On Dec 18, 2014 10:44 PM, "Hunter Leath" notifications@github.com wrote:

This wouldn't be related if the original error had to do with arena
allocation since the arena is only allocated once at the beginning of the
program, correct?
Correct. The arena will be allocated before executing any user Go code.

@crawshaw
Copy link
Member Author

crawshaw commented Jan 9, 2015

This change (324f38a) is small (the diff is mostly indenting) and important to starting Go apps on android. Could this be considered for backporting to 1.4.1?

Right now people are working off tip, which isn't a good idea given the state of the GC. And I suspect it will become even less of an option during the compiler transition.

@crawshaw crawshaw reopened this Jan 9, 2015
@ianlancetaylor
Copy link
Contributor

You don't need to reopen the issue. Simply adding the 1.4.1 milestone is enough for it to be considered for 1.4.1. Most of the 1.4.1 issues are closed.

@rsc
Copy link
Contributor

rsc commented Jan 14, 2015

@crawshaw I will grudgingly take this for 1.4.1 but in the future please do not mix code cleanup like variable renamings into subtle semantic changes.

@rsc rsc removed this from the Go1.4.1 milestone Jan 14, 2015
@rsc
Copy link
Contributor

rsc commented Jan 14, 2015

Doesn't apply cleanly (the Go source file edited in the commit was a C file in the release branch). Based on that and problems with the logging CL, I suggest posting github.com/crawshaw/go-android for people who want to follow along (or let them keep using 1.4).

@crawshaw
Copy link
Member Author

Acknowledged. Apologies for the variable rename, it slipped in when I copied a file in the middle of the code review.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants