Closed
Bug 1235733
Opened 8 years ago
Closed 8 years ago
Use absolute paths for include paths
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox46 fixed)
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file)
12.29 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
Limit ourselves to include paths for now, because there are tricky things involved in making this globally. While here, use shell_quote instead of manual quoting for those paths. I didn't check the size of the libxul dwarf info, but in terms of number of files reported in the crash reporter symbols, we go down from 22055 to 12470 on a linux64 build (numbers are similar on osx builds ; they don't change on windows for some reason, and the number of files is around 300k there to start with, so there's something else going on). That's because since we sometimes include with -I../dist/include, sometimes with -I../../dist/include, etc. we end up with many headers included with different paths, and referenced as many times. The worst one is nsIAtom.h, referenced 36 times! And for some reason, it's even worse for some files in the source directory (not entirely sure why), where the worst one is mozalloc.h, 293 times! By using absolute paths everywhere, we get down to 1 for each (except a few that still appear 2 or 4 times, not sure why, but that's surely much better)
Attachment #8702812 -
Flags: review?(gps)
Assignee | ||
Updated•8 years ago
|
Blocks: fast-compiledb
Comment 2•8 years ago
|
||
Comment on attachment 8702812 [details] [diff] [review] Use absolute paths for include paths Review of attachment 8702812 [details] [diff] [review]: ----------------------------------------------------------------- Nice wins on the debug info! I'm curious what the ccache/sccache implications are, if any. My understanding is the ccache mode for stripping prefixes doesn't really work that well and we're already busted. So, adding absolute paths to command arguments shouldn't impact caching (other than the one-time hit we take when this lands, which will prevent cache hits from anything before). You know more about this than me, so I trust your judgement on landing this.
Attachment #8702812 -
Flags: review?(gps) → review+
Comment 4•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cd0110d2278f
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•