Closed Bug 1235733 Opened 4 years ago Closed 4 years ago

Use absolute paths for include paths

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

No description provided.
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)
Blocks: 1235748
Depends on: 1235866
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+
https://hg.mozilla.org/mozilla-central/rev/cd0110d2278f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.