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: allow users to opt into 4MB arenas #42612

Open
calder opened this issue Nov 14, 2020 · 7 comments · May be fixed by #42611
Open

runtime: allow users to opt into 4MB arenas #42612

calder opened this issue Nov 14, 2020 · 7 comments · May be fixed by #42611
Assignees
Labels
Milestone

Comments

@calder
Copy link

@calder calder commented Nov 14, 2020

Problem

Some embedded Linux platforms disable overcommit to reduce dynamic behavior in the system. Go already uses 4MB arenas on 32-bit platforms, but on 64-bit platforms the initial 64MB arena adds a steep cost to any Go binary.

Proposal

Allow users to opt into 4MB arenas at build time by passing -tags force_small_arena to the compiler.

References

@calder calder linked a pull request that will close this issue Nov 14, 2020
@calder calder changed the title runtime: runtime: allow users to opt into 4MB arenas Nov 14, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 14, 2020

Change https://golang.org/cl/269998 mentions this issue: runtime: add force_small_arena build tag

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 14, 2020

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Nov 16, 2020

I don't think a build tag (or any optional behavior) is the right way to resolve running up against strict overcommit. It's another configuration to maintain.

I think we should consider doing a 64 MiB reservation (PROT_NONE, which doesn't count toward overcommit if it hasn't been touched before) and mapping pages read-write as needed. We already have a mechanism in the runtime to "grow" into an existing arena (see (*mheap).grow and mheap.curArena) we would just need to move the sysMap call in (*mheap).sysAlloc there instead. Then we get this behavior on every platform which could have similar restrictions for 64 MiB arenas.

Such a change does mean we can retain the performance benefits of having a shallower arena index on 64-bit platforms (1-2%), but where we lose is that we potentially have to do more syscalls. I personally don't think that's a big deal because heap growths are relatively rare whereas we access the arena index extremely often. Notably, we won't have to do more syscalls than on 4 MiB arena platforms since we also already round up ask in (*mheap).grow to pallocChunkPages (which happens to be 4 MiB).

It's a fairly small and easy change to make, though unfortunately I think at this point it will have to wait for Go 1.17, unless I'm wrong.

@aclements WDYT?

@aclements
Copy link
Member

@aclements aclements commented Nov 16, 2020

I agree that I don't want to ship a configuration flag for this. But I like @mknyszek's idea of using mheap.curArena to map the reservation more incrementally. This would also solve #28081 (see in particular #28081 (comment)). I think we have to be careful not to map it in too small of chunks just to reduce mmap overhead, but a few megs at a time ought to be plenty.

I think this is too late for 1.16, unfortunately, but since it should be a pretty simple change we could make it now and queue it up for 1.17. @mknyszek , care to do the honors?

@aclements
Copy link
Member

@aclements aclements commented Nov 16, 2020

(Actually, we should definitely do it at least 2 MiB at a time so we don't break up huge pages on amd64.)

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 16, 2020

Change https://golang.org/cl/270537 mentions this issue: runtime: prepare arenas for use incrementally

@mknyszek mknyszek self-assigned this Nov 16, 2020
@mknyszek mknyszek added the NeedsFix label Nov 16, 2020
@mknyszek mknyszek modified the milestones: Backlog, Go1.17 Nov 16, 2020
@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Nov 16, 2020

@calder if it's not too much trouble, do you mind trying out https://golang.org/cl/270537 and seeing if it resolves issues with strict overcommit for you?

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.

5 participants
You can’t perform that action at this time.