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

Closed
calder opened this issue Nov 14, 2020 · 7 comments
Closed

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

calder opened this issue Nov 14, 2020 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@calder
Copy link

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 changed the title runtime: runtime: allow users to opt into 4MB arenas Nov 14, 2020
@gopherbot
Copy link

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

@ianlancetaylor
Copy link
Contributor

CC @aclements @mknyszek

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 14, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Nov 14, 2020
@mknyszek
Copy link
Contributor

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

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

(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

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 The path to resolution is known, but the work has not been done. label Nov 16, 2020
@mknyszek mknyszek modified the milestones: Backlog, Go1.17 Nov 16, 2020
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 16, 2020
@mknyszek
Copy link
Contributor

@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?

@golang golang locked and limited conversation to collaborators Mar 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants