Last Comment Bug 721131 - Speed up build in content/svg/content/src - clean up #include directives in its header files
: Speed up build in content/svg/content/src - clean up #include directives in i...
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla12
Assigned To: Jonathan Watt [:jwatt]
: Jet Villegas (:jet)
Depends on:
Blocks: iwyu
  Show dependency treegraph
Reported: 2012-01-25 11:11 PST by Jonathan Watt [:jwatt]
Modified: 2013-08-11 03:18 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (80.08 KB, patch)
2012-01-25 11:12 PST, Jonathan Watt [:jwatt]
dholbert: review+
Details | Diff | Splinter Review

Description User image Jonathan Watt [:jwatt] 2012-01-25 11:11:48 PST
I've been experimenting to try and figure out to what extent our diabolically slow build times are due to bad #include directives. That experiment didn't get far enough to shed much light on the subject, and it's kind of gone on the back burner for now until the tools improve. Nevertheless, I have a patch that can land now to save repeating work later.

First, note that modern compilers do include guard detection, so they won't open a header file twice for a given compilation unit if it's not necessary. (Not quite as good as '#pragma once', but almost.) So multiple #include directives for the same header while compiling a source file doesn't slow the build down noticeably. What slows things down is including unnecessary headers (directly, or more significantly, indirectly).

The first step to reducing unnecessary includes is actually to add any necessary ones - i.e. all header files should include all the headers for all the symbols they use. That doesn't harm build time due to the include guard compiler optimization, and it clears the way to start removing unnecessary includes from files without having things break because required headers were being indirectly included.

This patch tackles the #include directives in the header files in content/svg/content/src. For me it reduces the time it takes to do a full rebuild of the source in that directory by about 2%. Nothing worth shouting about, but mostly its benefit is that it clears the way to clean up the includes in the .cpp files. Unfortunately the tool I've been playing with - include-what-you-use - segfaults on most of our .cpp files.

This patch also reorders the #include directives and the class/struct declarations in the header files to put them alphabetically (case insensitive). The include lines were getting enough churn that now was a good time to clean that up.
Comment 1 User image Jonathan Watt [:jwatt] 2012-01-25 11:12:40 PST
Created attachment 591553 [details] [diff] [review]
Comment 2 User image Daniel Holbert [:dholbert] 2012-01-25 13:50:07 PST
Comment on attachment 591553 [details] [diff] [review]

Looks good to me!

As discussed on IRC: if/when we eventually do this cleanup on .cpp files, I think we'll want to have an exception to the alphabetization, so that Foo.cpp includes Foo.h as its first include.  That doesn't affect this patch, though.
Comment 3 User image Marco Bonardo [::mak] 2012-01-26 15:57:48 PST

Note You need to log in before you can comment on or make changes to this bug.