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/go: add -debug flag (default true) to control DWARF/etc info in binary #38777

Open
josharian opened this issue Apr 30, 2020 · 17 comments
Open

Comments

@josharian
Copy link
Contributor

Many gophers know that they can strip DWARF from their binaries with -ldflags=-w. But this is fairly cryptic, and it doesn't give the compiler the opportunity to save time and space by not generating that DWARF in the first place.

I propose we add support directly to cmd/go to say -debug=false or -dwarf=false or the like. Then cmd/go would translate that into the appropriate compiler and linker flags.

@gopherbot gopherbot added this to the Proposal milestone Apr 30, 2020
@mvdan
Copy link
Member

mvdan commented May 1, 2020

I wonder if this would be a net win overall, assuming that using -ldflags=-w or not doesn't affect the compiler, so both versions can share the build cache entirely.

@mvdan
Copy link
Member

mvdan commented May 1, 2020

Here's one case where it could help build speed a lot, though - single production builds from scratch, such as those that happen inside Dockerfiles, or on CI/CD machines. Those often exclude debug information, and they rarely have a warm build cache, given that they're not developer machines. For the same reason, they tend to compile a lot of packages from scratch, so even saving 2% of the compiler's work would be noticeable.

@rsc rsc added this to Incoming in Proposals (old) Jun 10, 2020
@cherrymui
Copy link
Member

it doesn't give the compiler the opportunity to save time and space by not generating that DWARF in the first place.

The compiler has -dwarf=false flag.

@mvdan
Copy link
Member

mvdan commented Jun 12, 2020

The compiler has -dwarf=false flag.

I think that's what @josharian meant by "Then cmd/go would translate that into the appropriate compiler and linker flags".

Also, I don't think it's quite as simple as doing go build -gcflags=all=-dwarf=false -ldflags=-w. Imagine the case where most of the library packages have already been built with the regular go build, and we just need to re-compile a few packages and re-link the entire binary. -gcflags=all=-dwarf=false will probably force recompiling all the dependencies, defeating the entire purpose of avoiding work in the first place.

I think go build -debug=false could be smarter in that way. For packages it does compile, it would know it can add flags like -dwarf=false to avoid extra work. Similarly, for link operations it would add -w. But it would not need to re-compile any package that was previously compiled with debug information, as it knows that the linker's -w will discard that anyway.

@rsc rsc moved this from Incoming to Active in Proposals (old) Jul 8, 2020
@rsc
Copy link
Contributor

rsc commented Jul 15, 2020

@mvdan:

I think go build -debug=false could be smarter in that way. For packages it does compile, it would know it can add flags like -dwarf=false to avoid extra work. Similarly, for link operations it would add -w. But it would not need to re-compile any package that was previously compiled with debug information, as it knows that the linker's -w will discard that anyway.

FWIW, I don't think we can do that. I don't know how the go command would compute accurate build IDs that would come out the same for a build in which some inputs have expected-to-be-ignored DWARF info versus a build in which no inputs do. Those are different incoming file hashes, and I don't see how to generate the outgoing file hash in a way that gets the same answer in both cases. Builds will become nondeterministic based on the content of the cache. And debugging places where DWARF info accidentally does change other compile results will become very difficult. Even if you somehow know the set of inputs to use, deciding whether you have a cache hit would now require two lookups - one to check for a -dwarf=false build and one to check for a -dwarf=true build. It's all very complicated and not obviously worth the complexity.

So if we make -debug=false mean -gcflags=all=-dwarf=false -ldflags=-w, I think it basically has to mean a full recompilation, as you noted. Whether that's worthwhile is an open question, I think. My instinct is "probably not worthwhile", but maybe the first rebuild is not such a big deal and then everything is cached.

In contrast, if -debug=false meant only -ldflags=-w, that would be a clear win and have no caching complexity - do we know what percentage of compile time is spent on DWARF, versus what percentage of link time?

If the main purpose of the flag is to be a clearer statement of intent, then only applying to the link step might be fine: -debug=false is much clearer than -ldflags=-w, and it could be made to work no matter what the underlying toolchain, making it a portability win too.

@josharian
Copy link
Contributor Author

do we know what percentage of compile time is spent on DWARF

IIRC 1–2%

versus what percentage of link time?

I don’t know this number offhand, but I think people mainly disable DWARF for smaller binaries, not faster link (or compile) times. Faster builds is an accidental benefit.

it basically has to mean a full recompilation

I’m not so sure. ‘go test’ builds without DWARF, so in the common case in which people run tests before generating a binary (both locally and in CI/CD), it might actually be the DWARF-containing build that requires significant recompilation.

If the main purpose of the flag is to be a clearer statement of intent

FWIW, a clearer statement of intent was my primary goal in filing this proposal. But as per above, I think we can also get faster builds.

@mvdan
Copy link
Member

mvdan commented Jul 18, 2020

FWIW, I don't think we can do that.

@jayconrod brought up the same point on the golang-tools call a few weeks ago, and it's true that I wasn't thinking of the pitfalls of forcing the build cache this way.

@rsc
Copy link
Contributor

rsc commented Jul 22, 2020

@josharian

I’m not so sure. ‘go test’ builds without DWARF, so in the common case in which people run tests before generating a binary (both locally and in CI/CD), it might actually be the DWARF-containing build that requires significant recompilation.

Looking at the code, it looks like OmitDebug is only set for the package main in a test or in "go run", not recursively down the tree. So I don't think those are doing full rebuilds necessarily. Would be nice if we were already doing full recursive non-DWARF builds, of course, but I don't think we are. (There are also implications for using the pre-cgo-ed packages like net.a without a C compiler toolchain installed if we do the recursive no-dwarf.)

It still seems like the linker is where the win mostly is.

Does anyone want to gather numbers about how much speedup there is in compiler + linker, vs how much recompilation is incurred?

@rsc
Copy link
Contributor

rsc commented Aug 5, 2020

Discussion here seems blocked on someone gathering specific numbers showing that adding this flag would be helpful in practice. Anyone interested?

@cherrymui
Copy link
Member

I just wanted to mention that in the past we started with nearly all DWARF generation in the linker, and we have moved a good portion to the compiler, and we are planning to move more. (Not sure how this will affect the decision here.)

@josharian
Copy link
Contributor Author

Discussion here seems blocked on someone gathering specific numbers showing that adding this flag would be helpful in practice.

I can gather numbers, but it probably won't happen particularly soon.

But to re-iterate, I think the primary value here is clarity, and performance benefits are secondary.

@rsc rsc changed the title proposal: cmd/go: add -debug or -dwarf flag proposal: cmd/go: add -debug flag (default true) to control DWARF/etc info in binary Aug 12, 2020
@rsc
Copy link
Contributor

rsc commented Aug 12, 2020

I see the clarity increase for Go developers who want to build a binary without DWARF for size reasons.

I suppose the performance numbers would let us decide whether to make -debug=false (not DWARF - there are other kinds of debug info on other systems) tell that to just the linker or also the compiler. The user doesn't care - same result either way.

Based on the conversation above, then, this seems like a likely accept with the caveat that we don't know exactly what the implementation will be.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Aug 14, 2020
@josharian
Copy link
Contributor Author

Disabling DWARF generation in the compiler speeds up compilation and reduces memory use by about 3% each.

With HEAD at fb5c3ea, I ran compilecmp -n 5 -afterflags=-dwarf=false head head to gather memory impact and compilecmp -n 100 -cpu -afterflags=-dwarf=false head head to gather execution time impact. Combined results:

name        old time/op       new time/op       delta
Template          194ms ± 2%        187ms ± 2%  -3.20%  (p=0.000 n=95+96)
Unicode          82.0ms ± 4%       81.0ms ± 4%  -1.21%  (p=0.000 n=97+96)
GoTypes           666ms ± 3%        646ms ± 3%  -3.11%  (p=0.000 n=97+95)
Compiler          3.19s ± 2%        3.07s ± 2%  -3.91%  (p=0.000 n=94+95)
SSA               7.42s ± 2%        6.99s ± 2%  -5.73%  (p=0.000 n=98+97)
Flate             127ms ± 2%        123ms ± 2%  -3.03%  (p=0.000 n=92+93)
GoParser          155ms ± 2%        150ms ± 1%  -3.32%  (p=0.000 n=95+95)
Reflect           418ms ± 3%        409ms ± 4%  -2.21%  (p=0.000 n=95+97)
Tar               172ms ± 2%        166ms ± 2%  -3.22%  (p=0.000 n=94+97)
XML               231ms ± 2%        222ms ± 3%  -3.71%  (p=0.000 n=95+93)
[Geo mean]        390ms             377ms       -3.27%

name        old user-time/op  new user-time/op  delta
Template          235ms ± 7%        221ms ± 2%  -5.83%  (p=0.000 n=100+80)
Unicode           110ms ± 4%        109ms ± 4%  -1.27%  (p=0.000 n=98+97)
GoTypes           830ms ± 3%        832ms ± 2%    ~     (p=0.139 n=98+95)
Compiler          4.14s ± 2%        4.11s ± 2%  -0.73%  (p=0.000 n=100+87)
SSA               10.1s ± 2%         9.3s ± 1%  -7.75%  (p=0.000 n=98+98)
Flate             150ms ±14%        145ms ± 7%  -3.56%  (p=0.000 n=100+100)
GoParser          184ms ± 2%        177ms ± 2%  -4.24%  (p=0.000 n=97+96)
Reflect           507ms ± 3%        514ms ± 4%  +1.28%  (p=0.000 n=96+98)
Tar               208ms ± 2%        200ms ± 2%  -3.92%  (p=0.000 n=97+94)
XML               285ms ± 2%        271ms ± 2%  -4.81%  (p=0.000 n=96+99)
[Geo mean]        487ms             472ms       -3.10%

name        old alloc/op      new alloc/op      delta
Template         34.8MB ± 0%       33.9MB ± 0%  -2.47%  (p=0.008 n=5+5)
Unicode          29.3MB ± 0%       29.1MB ± 0%  -0.84%  (p=0.008 n=5+5)
GoTypes           115MB ± 0%        111MB ± 0%  -3.25%  (p=0.008 n=5+5)
Compiler          553MB ± 0%        528MB ± 0%  -4.50%  (p=0.008 n=5+5)
SSA              1.32GB ± 0%       1.28GB ± 0%  -2.84%  (p=0.008 n=5+5)
Flate            21.8MB ± 0%       21.4MB ± 0%  -2.15%  (p=0.008 n=5+5)
GoParser         26.7MB ± 0%       26.1MB ± 0%  -2.25%  (p=0.008 n=5+5)
Reflect          75.0MB ± 0%       73.1MB ± 0%  -2.46%  (p=0.008 n=5+5)
Tar              32.6MB ± 0%       31.8MB ± 0%  -2.44%  (p=0.008 n=5+5)
XML              41.5MB ± 0%       39.7MB ± 0%  -4.49%  (p=0.008 n=5+5)
[Geo mean]       74.2MB            72.1MB       -2.78%

name        old allocs/op     new allocs/op     delta
Template           336k ± 0%         326k ± 0%  -2.99%  (p=0.008 n=5+5)
Unicode            338k ± 0%         337k ± 0%  -0.26%  (p=0.008 n=5+5)
GoTypes           1.17M ± 0%        1.13M ± 0%  -2.96%  (p=0.008 n=5+5)
Compiler          4.92M ± 0%        4.76M ± 0%  -3.24%  (p=0.008 n=5+5)
SSA               12.2M ± 0%        11.8M ± 0%  -3.13%  (p=0.008 n=5+5)
Flate              215k ± 0%         207k ± 0%  -3.52%  (p=0.008 n=5+5)
GoParser           270k ± 0%         263k ± 0%  -2.58%  (p=0.008 n=5+5)
Reflect            877k ± 0%         856k ± 0%  -2.42%  (p=0.008 n=5+5)
Tar                313k ± 0%         301k ± 0%  -3.86%  (p=0.008 n=5+5)
XML                387k ± 0%         375k ± 0%  -3.18%  (p=0.008 n=5+5)
[Geo mean]         739k              718k       -2.82%

@rsc
Copy link
Contributor

rsc commented Aug 26, 2020

Thanks for the numbers, @josharian.

I still wonder a bit about how much the overhead of a full rebuild with dwarf info later matters but maybe most people are just repeating one particular go command (like go test or go run), in which case you get a cached build on all the repeats anyway.

@rsc
Copy link
Contributor

rsc commented Aug 26, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Aug 26, 2020
@thanm
Copy link
Contributor

thanm commented Aug 26, 2020

In the C/C++ compiler world, one thing that I've seen is that the presence or absence of "-g" can cause unexpected differences in the code generated by the compiler (e.g. "cc -g -O myfile.c" produces different assembly from "cc -O myfile.c"). This has come up many times in nearly every compiler I've worked on.

Once we have "-debug={true,false}" up and running for the Go compiler, I think it would be useful to enhance our tools testing to:

  • add some sort of compatibility comparable to "toolstash -check" that verifies that code generated with and without "-debug=..." is the same (to be used by compiler developers in pre-submit testing)

  • add some set of builders that enforce the same invariant

My 2 cents.

@ianlancetaylor
Copy link
Contributor

GCC has a -fcompare-debug option that automatically runs the compiler twice, once with debug info and once without, and does a comparison of the final compiler IR from both runs, ignoring debug info, and complains if they are different.

@rsc rsc modified the milestones: Proposal, Backlog Aug 26, 2020
@rsc rsc changed the title proposal: cmd/go: add -debug flag (default true) to control DWARF/etc info in binary cmd/go: add -debug flag (default true) to control DWARF/etc info in binary Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

7 participants