Page MenuHomePhabricator

[gn build] Link with -Wl,--gdb-index
ClosedPublic

Authored by thakis on Dec 8 2020, 6:43 AM.

Details

Summary

For full-debug-info (is_debug=true / symbol_level=2 builds), this makes
linking 15% slower, but gdb startup 1500% faster (for lld: link time
3.9s->4.4s, gdb load time >30s->2s).

For link time, I ran

bench.py -o {noindex,index}.txt \
    sh -c 'rm out/gn/bin/lld && ninja -C out/gn lld'

and then ministat noindex.txt index.txt:

x noindex.txt
+ index.txt
    N           Min           Max        Median           Avg        Stddev
x   5      3.784461     4.0200169     3.8452811     3.8754988   0.089902595
+   5       4.32496     4.6058481     4.3361208     4.4141198    0.12288267
Difference at 95.0% confidence
        0.538621 +/- 0.15702
        13.8981% +/- 4.05161%
        (Student's t, pooled s = 0.107663)

For gdb load time I loaded the crash in PR48392 with

gdb -ex r --args ../out/gn/bin/ld64.lld.darwinnew @response.txt

and just stopped the time until the crash got displayed with a stopwatch
a few times. So the speedup there is less precise, but it's so
pronounced that that's ok (loads ~instantly with the patch, takes a very
long time without it).

Only doing this for LLD because I haven't tried it with other linkers.


Not sure if anyone cares about this. Holler if you think we shouldn't do this and/or put it behind a flag. See comment in the build file for the reasoning why I think this is a good default.

Diff Detail

Event Timeline

thakis created this revision.Dec 8 2020, 6:43 AM
thakis requested review of this revision.Dec 8 2020, 6:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2020, 6:43 AM
hans accepted this revision.Dec 8 2020, 6:56 AM
This revision is now accepted and ready to land.Dec 8 2020, 6:56 AM

Hm D58628 suggests that maybe bfd ld doesn't support this. So maybe we should only do it if lld? (There's no "use_gold" var and I'd rather not add one.)

thakis updated this revision to Diff 310186.Dec 8 2020, 7:11 AM
thakis edited the summary of this revision. (Show Details)

lld only

I'm confused on the term "release build". I typically use is_optimized = true and is_debug = true to get reasonably fast binaries but with backtraces for crashes (I work with a lot of crashes). Which = false does a "release build" refer to?

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

I'm confused on the term "release build". I typically use is_optimized = true and is_debug = true to get reasonably fast binaries but with backtraces for crashes (I work with a lot of crashes). Which = false does a "release build" refer to?

Will replace "release" with "no-debug-information".

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

You're setting is_debug=true but you don't run the binary under gdb? If so, wy do you set is_debug=true? For stacks with line numbers?

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

You're setting is_debug=true but you don't run the binary under gdb? If so, wy do you set is_debug=true? For stacks with line numbers?

Yup

MaskRay added a subscriber: MaskRay.Dec 9 2020, 9:13 AM

If -gsplit-dwarf is set, -Wl,--gdb-index makes sense.

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

You're setting is_debug=true but you don't run the binary under gdb? If so, wy do you set is_debug=true? For stacks with line numbers?

Yup

Do you think we should have some mode to get -g1 for that use case? I'd expect that to make your use case link faster than it currently does. (At least while we don't use fission, but even then it'll reduce .o file size which likely means faster build time with distributed compiles.) And then we could do the index only on -g not -g1 builds.

If -gsplit-dwarf is set, -Wl,--gdb-index makes sense.

The GN build currently never sets gsplit-dwarf and -Wl,--gdb-index still seems to speed up things quite a bit. Are you saying we should _also_ use -gsplit-dwarf (probably a good idea), or are you saying something else?

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

You're setting is_debug=true but you don't run the binary under gdb? If so, wy do you set is_debug=true? For stacks with line numbers?

Yup

Do you think we should have some mode to get -g1 for that use case? I'd expect that to make your use case link faster than it currently does. (At least while we don't use fission, but even then it'll reduce .o file size which likely means faster build time with distributed compiles.) And then we could do the index only on -g not -g1 builds.

Oh yes that would be ideal.

15% slower links seems noticeable. I'd prefer putting this behind a flag (can default to is_debug if you really want). I think putting all optional compiler/linker command line flags behind gn args makes sense.

You're setting is_debug=true but you don't run the binary under gdb? If so, wy do you set is_debug=true? For stacks with line numbers?

Yup

Do you think we should have some mode to get -g1 for that use case? I'd expect that to make your use case link faster than it currently does. (At least while we don't use fission, but even then it'll reduce .o file size which likely means faster build time with distributed compiles.) And then we could do the index only on -g not -g1 builds.

If -gsplit-dwarf is set, -Wl,--gdb-index makes sense.

The GN build currently never sets gsplit-dwarf and -Wl,--gdb-index still seems to speed up things quite a bit.

Yep - gdb-index should still be a significant speed-up even without split-dwarf. (I think there was some discussion elsewhere (on a bug or the like), but you are enabling gnu pubnames (-ggnu-pubnames) on your compiles along with gdb-index, yeah?)

Are you saying we should _also_ use -gsplit-dwarf (probably a good idea), or are you saying something else?

gdb-index doesn't need split-dwarf, but split-dwarf (at least with GDB) does need gdb-index, FWIW

If split-dwarf works for the build system (in terms of having the .dwo files available - if there's an distributed element, then that distributed system needs to be aware of the dwo files and be able to move them back - and the pathnames in the .o files need to be created in such a way that the debugger can find the dwo files after they've been moved around, etc) then, yeah, there's some benefits to be had there - smaller linker input/outputs and so some speedups.

thakis edited the summary of this revision. (Show Details)Dec 17 2020, 11:28 AM
thakis edited the summary of this revision. (Show Details)

Do you think we should have some mode to get -g1 for that use case? I'd expect that to make your use case link faster than it currently does. (At least while we don't use fission, but even then it'll reduce .o file size which likely means faster build time with distributed compiles.) And then we could do the index only on -g not -g1 builds.

Oh yes that would be ideal.

I rebased this across your CL (not yet uploaded) and measured --gdb-index impact for -g1:

    N           Min           Max        Median           Avg        Stddev
x   5    0.91551208     1.0278211    0.99482608    0.98919582    0.04413594
+   5     1.0646391      1.082478     1.0822251      1.076741   0.008073845
Difference at 95.0% confidence
	0.0875452 +/- 0.0462716
	8.85014% +/- 4.6777%
	(Student's t, pooled s = 0.0317267)

So with -g1 it still makes likes ~9% slower, but the absolute time difference for lld is < 0.1s, instead of 0.5s for -g2. (Load time in gdb in both cases is < 1s, with and without index. With index feels maybe a bit faster, but it's fast enough anyways that it isn't a dramatic benefit.) Do you have an opinion on if we should pass this for g1 too? Benefit is a slightly more regular build configuration, drawback is very slightly slower links. Let me know what you prefer :)

thakis edited the summary of this revision. (Show Details)

rebase, and only for full debug info

<0.1s seems pretty unnoticeable.
But does anybody use debuggers with -g1? If not then I don't see any reason to pass --gdb-index.

aeubanks accepted this revision.Dec 17 2020, 11:47 AM