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

A minimal rebuild of pdb-downloader takes too long #47240

Closed
jrmuizel opened this issue Jan 6, 2018 · 15 comments
Closed

A minimal rebuild of pdb-downloader takes too long #47240

jrmuizel opened this issue Jan 6, 2018 · 15 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-compiletime Issue: Problems and improvements with respect to compile times. O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jrmuizel
Copy link
Contributor

jrmuizel commented Jan 6, 2018

On my MacBook Pro I see:

$ touch main.rs
$ CARGO_INCREMENTAL=1 cargo build
   Compiling pdb-downloader v0.1.0 (file:///Users/jrmuizel/tools/pdb-downloader)
    Finished dev [unoptimized + debuginfo] target(s) in 2.67 secs

main.rs is only 90 lines

@Mark-Simulacrum
Copy link
Member

Is this the project? https://github.com/jrmuizel/pdb-downloader/

@Mark-Simulacrum Mark-Simulacrum added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 6, 2018
@jrmuizel
Copy link
Contributor Author

jrmuizel commented Jan 6, 2018

Ah yes, sorry. I meant to include that link in the original report.

@Thiez
Copy link
Contributor

Thiez commented Jan 7, 2018

Looks to me like over half the time is spent on running the linker. Which doesn't surprise me, the dependencies are huge! In a debug-build, goblin is a 17MB rlib, indicativ is a 6.2MB rlib, and reqwest is a 28MB rlib. I'm guessing your compile times wouldn't be must better in C or C++.

If rust/cargo had better support for dynamic linking it might go a lot faster.

@alexcrichton
Copy link
Member

Some local timings:

  • 2.9s overall
    • 0.6s for running cc (the linker)
    • ~1.2s for running dsymutil (had to guess, this isn't timed by rustc by default)
    • 0.5s for all of translation (with a fully warm incremental dir)

Opening up a profiler I see

  • 7.6% of time is spent in DepGraph::try_mark_green
  • 5.9% of time is spent in the link function (the libc function link)
  • 4.7% is memmove
  • 4.1% is mmap, mostly from jemalloc

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Jan 9, 2018

Given the object files should still be in place is it possible for us to avoid having to run dsymutil?

@alexcrichton
Copy link
Member

So I, personally, actually have no idea why we run dsymutil. I did a bit of digging to find out and I've never blamed a line of code so far back at this one in Rust's history. Turns out 196351aa introduced it, and as with many lines of code written seven (!) years ago it's changed a lot since then and unfortunately doesn't have a ton of rationale for why it exists (hey one can hope can't he?).

That being said a little testing locally makes it pretty obvious as to why it exists. At least lldb doesn't work without it! Or at least for me if I deleted the *.dSYM folder then lldb wasn't able to set a breakpoint. It looks as if dsymutil doesn't modify the binary as well, so I think it's just generating that extra debuginfo.

Despite that though I still have very little idea what dsymutil is doing. For example are there options we can compile with to make it take less time? Options to dsymutil? Something like -gsplit-dwarf to make it (I guess?) copy less data around? It seems to spend a lot fo dime in DwarfLinker::DIECloner (at least the LLVM implementation which I assume is the same as Apple's at some point). The source doens't seem too illuminating.

My guess is that we can't not run dsymutil if we want up-to-date debug information, but I could be wrong! With all the work being done it seems unlikely at least that lldb is going back to the original objects.

All that being said surely C++ runs into similar problems here. Or maybe we generate more debuginfo? Or maybe the executable here is too big? Unsure :(

@alexcrichton
Copy link
Member

Interesting though!

So rustc deletes object files that go into rlibs, basically cleaning up after itself. If we don't do that, however, then lldb appears to work even if the *.dSYM is deleted.

And then even more interestingly!

It turns out that if I delete the *.dSYM folder then lldb does actually work for dependencies. That actually all makes sense to me I think. I believe that dsymutil is basically a "dwarf linker" which presumably just assembles everything into one location (the *.dSYM folder). The actual binary itself just contains a bunch of mappings to "go find my debuginfo in these files" (I believe) and lldb seems to process those if *.dSYM isn't available (not sure of the priority, but hey).

So debugging a binary doesn't actually work because the object files for the binary are cleaned up and deleted by rustc. Debugging a library of a binary works though because the rlib is still on the filesystem after the build.

It seems odd to try to change course with dsymutil now but it actually seems like we should:

  • Not delete object files for binaries with debuginfo (somehow)
  • Not run dsymutil by default if this happens.

I believe that means that we wouldn't run dsymutil at all by default as part of rustc itself. This should save time on incremental builds as we don't ever run anything, and if someone's build system needs to ship the *.dSYM folder they can always generated the debug symbols themselves by running dsymutil as a postprocessing step.

Now emitting object files from rustc by default is pretty tricky. Mostly in that we've never done it before and it's not clear if people want object files getting jettisoned out. What I think we could do though is not change rustc's defaults but change Cargo's defaults. For example we could have a flag to rustc like --leave-debuginfo-related-files-on-disk-i-am-managing-them and Cargo would pass that along. When rustc has that flag it wouldn't execute dsymutil, making the incremental builds here 38% faster! (ish)

@michaelwoerister curious what you think about this! I feel like it'd actually be pretty nice to not have to run dsymutil...

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Jan 9, 2018

@luser

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Jan 9, 2018

Yeah, we don't run dsymutil on the c++ part of Firefox builds because we still have all of the object files around.

@michaelwoerister
Copy link
Member

Keeping object files around would also help on Linux where we could use debuginfo fission to speed up linking. The same approach might be possible on OS X.

@luser
Copy link
Contributor

luser commented Jan 9, 2018

I believe that dsymutil is basically a "dwarf linker" which presumably just assembles everything into one location (the *.dSYM folder)

Yes, you're exactly correct. Apple's toolchain does the equivalent of -gsplit-dwarf by default, and dsymutil links the debug info. Since rustc runs dsymutil after every link it effectively nullifies the benefits of not linking the DWARF. :) I'm not surprised that lldb supports locating the object files and reading the debug info from there, Apple's gdb supported that as well. (FWIW, the stabs entries in the binaries that lldb/gdb use to locate the object files are the same ones causing the non-determinism you found!)

dsymutil is still useful because most other tools do not know how to locate the debug symbols from the object files, so people who want to run things like profilers or whatnot will probably need this step. It's not particularly onerous to tell people that they have to run dsymutil target/whatever/yourbinary to make those use cases work, though, especially if the tradeoff is much faster builds.

Just as an aside, dsymutil takes forever to run on Firefox's libxul. We don't run it as part of the normal build, but we run it as a post-build step so we can upload the dSYM to our symbol server and also so we can run our dump_syms tool on it to get symbols in Breakpad format for our crash reports. If we ever get to a point where we let cargo build libxul we'd definitely want this fixed!

@jrmuizel
Copy link
Contributor Author

jrmuizel commented Jan 9, 2018

FWIW, Instruments does not need you to run dysmutil to profile Firefox. I'm not sure what tools do.

@alexcrichton
Copy link
Member

I've got an initial pass for adding an option to not run dsymutil at #47784, and once that lands I plan to update Cargo to by default not run dsymutil

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 12, 2018
This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
kennytm added a commit to kennytm/rust that referenced this issue Feb 13, 2018
…elwoerister

rustc: Add the ability to not run dsymutil

This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
kennytm added a commit to kennytm/rust that referenced this issue Feb 14, 2018
…elwoerister

rustc: Add the ability to not run dsymutil

This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
@XAMPPRocky XAMPPRocky added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Apr 10, 2018
@mstange
Copy link

mstange commented Mar 10, 2022

Is this fixed now, now that dsymutil is no longer run by default?

@Mark-Simulacrum
Copy link
Member

A trivial rebuild takes 0.18s today on my Mac, and a from-scratch build including all dependencies takes ~3.13 seconds. I think this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-compiletime Issue: Problems and improvements with respect to compile times. O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants