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

cmd/link: linker memory use increase from 1.10 to 1.11 #26186

Closed
thanm opened this issue Jul 2, 2018 · 17 comments
Closed

cmd/link: linker memory use increase from 1.10 to 1.11 #26186

thanm opened this issue Jul 2, 2018 · 17 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@thanm
Copy link
Contributor

thanm commented Jul 2, 2018

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

tip:
go version devel +1e1bd239c5 Thu Jun 28 16:10:19 2018 -0400 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

When I build large Go programs, I note that memory consumption has increased during the link step, when moving from Go 1.10 to Go 1.11; it seems that the linker is taking more memory than it used to. The memory consumption increase varies from program to program, but looks on the order of 20-30%. Building "hyperkube" from kubernetes tip (1.11) is one example. The 20% number is is derived from building a specific copy of "hyperkube" with 1.10 & 1.11 and comparing heap profiles collected via

go build -ldflags "-memprofile XXX -memprofilerate 1"

using "go tool pprof". Link steps that use the external linker (for CGO) have larger increases.

The current theory is that this is related to additional DWARF symbols/sections/relocations that the compiler is emitting (for example, DWARF location lists are now enabled with 1.11, as are DWARF inline info records).

It would be worth doing some work to figure out exactly where the memory increase is coming from, and submitting fixes to bring down the overall usage if possible, so as to avoid having the linker run out of memory when linking very large Go programs.

@thanm thanm added this to the Go1.11 milestone Jul 2, 2018
@thanm thanm self-assigned this Jul 2, 2018
@ALTree ALTree changed the title linker memory use increase from 1.10 to 1.11 cmd/link: linker memory use increase from 1.10 to 1.11 Jul 2, 2018
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/121915 mentions this issue: cmd/link: use side table instead of sym.Symbol 'Reachparent' field

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/121916 mentions this issue: cmd/link: split off 'Dynimp' string fields to reduce sym.Symbol size

@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 3, 2018
gopherbot pushed a commit that referenced this issue Jul 3, 2018
The sym.Symbol 'Reachparent' field is used only when field tracking
is enabled. So as to use less memory for the common case where
field tracking is not enabled, remove this field and use a side
table stored in the context to achieve the same functionality.

Updates #26186

Change-Id: Idc5f8b0aa323689d4d51dddb5d1b0341a37bb7d2
Reviewed-on: https://go-review.googlesource.com/121915
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Jul 3, 2018
The linker's sym.Symbol struct contains two string fields, "Dynimplib"
and "Dynimpvers" that are used only in very specific circumstances
(for many symbols, such as DWARF syms, they are wasted space). Split
these two off into a separate struct, then point to an instance of
that struct when needed. This reduces the size of sym.Symbol so as to
save space in the common case.

Updates #26186

Change-Id: Id9c74824e78423a215c8cbc105b72665525a1eff
Reviewed-on: https://go-review.googlesource.com/121916
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Jul 3, 2018
The sym.Symbol 'Reachparent' field is used only when field tracking
is enabled. So as to use less memory for the common case where
field tracking is not enabled, remove this field and use a side
table stored in the context to achieve the same functionality.

Updates #26186

Change-Id: Idc5f8b0aa323689d4d51dddb5d1b0341a37bb7d2
Reviewed-on: https://go-review.googlesource.com/121915
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Jul 3, 2018
The linker's sym.Symbol struct contains two string fields, "Dynimplib"
and "Dynimpvers" that are used only in very specific circumstances
(for many symbols, such as DWARF syms, they are wasted space). Split
these two off into a separate struct, then point to an instance of
that struct when needed. This reduces the size of sym.Symbol so as to
save space in the common case.

Updates #26186

Change-Id: Id9c74824e78423a215c8cbc105b72665525a1eff
Reviewed-on: https://go-review.googlesource.com/121916
Reviewed-by: Ian Lance Taylor <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/122475 mentions this issue: cmd/link: improve allocation performance in relocsym

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/122482 mentions this issue: cmd/link: improve allocation performance in relocsym

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/121198 mentions this issue: cmd/link: mmap object data

@ianlancetaylor
Copy link
Member

Should we move this issue's milestone to 1.12 at this point?

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/123296 mentions this issue: cmd/link: no need to generate type attr for DWARF functype

@thanm
Copy link
Contributor Author

thanm commented Jul 13, 2018

Should we move this issue's milestone to 1.12 at this point?

I suppose that make sense. Done.

@thanm thanm modified the milestones: Go1.11, Go1.12 Jul 13, 2018
gopherbot pushed a commit that referenced this issue Jul 13, 2018
The linker's DWARF generation occasionally computes and attaches an
attribute X to a type even though the type's abbrev doesn't have the
specified attr. For example, the DW_TAG_subroutine_type abbrev entry
has no type attribute, but a type attr is given to it (wasting
memory). Similarly there are some places where a byte size attr is
added to a DIE whose abbrev lacks that attr. This patch trims away a
few of these not-needed attrs, saving some very tiny amount of memory.

Updates #26186

Change-Id: I69e853df468ac54b07772a614b4106d7c4dae01d
Reviewed-on: https://go-review.googlesource.com/123296
Reviewed-by: Heschi Kreinick <[email protected]>
Run-TryBot: Heschi Kreinick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/125476 mentions this issue: cmd/link: split out Extname into cold portion of sym.Symbol

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/125477 mentions this issue: cmd/link: move Localentry field in sym.Symbol to cold section

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/125478 mentions this issue: cmd/link: move Plt, Got fields in sym.Symbol to cold section

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/125479 mentions this issue: cmd/link: move ElfType field in sym.Symbol to cold section

@alevchuk
Copy link

I ran into this issue. Not able to build go 1.11 on raspberry pi with 433 MB of RAM: #26523 , go 1.10 builds fine

gopherbot pushed a commit that referenced this issue Aug 30, 2018
Create a new "AuxSymbol" struct into which 'cold' or 'infrequently
set' symbol fields are located. Move the Extname field from the
main Symbol struct to AuxSymbol.

Updates #26186

Change-Id: I9e795fb0cc48f978e2818475fa073ed9f2db202d
Reviewed-on: https://go-review.googlesource.com/125476
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
gopherbot pushed a commit that referenced this issue Aug 30, 2018
The sym.Symbol 'Localentry' field is used only with cgo and/or
external linking on MachoPPC. Relocate it to sym.AuxSymbol since it is
infrequently used, so as to shrink the main Symbol struct.

Updates #26186

Change-Id: I5872aa3f059270c2a091016d235a1a732695e411
Reviewed-on: https://go-review.googlesource.com/125477
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Aug 30, 2018
The sym.Symbol 'Plt' and 'Got' field are used only with cgo and/or
external linking and are not needed for most symbols. Relocate them to
sym.AuxSymbol so as to shrink the main Symbol struct.

Updates #26186

Change-Id: I170d628a760be300a0c1f738f0998970e91ce3d6
Reviewed-on: https://go-review.googlesource.com/125478
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Aug 30, 2018
The sym.Symbol 'ElfType' field is used only for symbols corresponding
to things in imported shared libraries, hence is not needed in the
common case. Relocate it to sym.AuxSymbol so as to shrink the main
Symbol struct.

Updates #26186

Change-Id: I803efc561c31a0ca1d93eca434fda1c862a7b2c5
Reviewed-on: https://go-review.googlesource.com/125479
Reviewed-by: Ian Lance Taylor <[email protected]>
@goenning
Copy link

goenning commented Oct 5, 2018

I think we've hit the same wall. I can't make our CircleCI build to pass with Go 1.11.1 due to link failures

/usr/local/go/pkg/tool/linux_amd64/link: signal: killed

Edit: I found a workaround by using -p=8 to limit number of concurrent processes.

@mark-rushakoff
Copy link
Contributor

In the CircleCI case in particular, through https://groups.google.com/forum/?pli=1#!topic/golang-nuts/E9viXwCsHJA with @ianlancetaylor's help I found out that when running on Docker in Circle, Go thinks there are 36 CPUs, which means the default number of parallel build processes will be 36, so it's not very surprising to run into OOM errors.

According to https://circleci.com/docs/2.0/configuration-reference/#resource_class, the default Docker container only has 2 CPUs available. So -p=8 is probably reasonable, but maybe dropping to -p=2 could be necessary.

@ianlancetaylor
Copy link
Member

@thanm Looks like most of the CLs attached to this issue have been submitted. Is there anything more to do here?

@thanm
Copy link
Contributor Author

thanm commented Dec 8, 2018

Yes, I think we should close it out. Austin has been collecting a list of more heavyweight things to do to improve linker performance (time and space) for post 1.12 development.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants