Closed
Bug 64023
Opened 24 years ago
Closed 11 years ago
reduce header #include dependencies to speed up compilation
Categories
(Core :: General, defect, P3)
Core
General
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: dbaron, Assigned: dbaron)
References
(Blocks 1 open bug)
Details
(Whiteboard: [xptest])
Attachments
(18 files)
There are a number of places where commonly included header files include lots
of other header files. This slows down compilation tremendously. It would be
great if we could greatly reduce the number of header files needed for compilation.
I've found a few specific problems already by looking through gcc-generated
.deps directories. Other than the ones fixed by the patch I'm about to attach,
these are:
nsTraceRefcnt.h includes <stdio.h> (should be in nsTraceRefcnt.cpp,
but will cause cascade of problems...)
nsString2.h (and others) include nsCRT.h
nsCRT.h includes nsCppSharedAllocator.h (for little reason)
[scc says to change to nsMemory::Free rather than delete[] ]
(COST: 0.2s)
nsGUIEvent.h includes nsIDOMKeyEvent.h
(COST: 2.3s)
nsNetUtil.h drags in a *lot* (see nsGlobalWindow.cpp)
(COST: 4.1s)
nsIPref.h includes jsapi.h
nsISupportsUtils brings in pratom.h, etc. (for threadsafety) even when unused
(hard to fix)
nsIAtom includes nsAWritableString.h (not really fixable, but costly)
prprf.h -> prio.h -> prinet.h -> *lots*
dom:
nsHistory.h includes nsISHistory.h
Assignee | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
Actually, lots of very common header files #include <stdio.h>.
Assignee | ||
Comment 3•24 years ago
|
||
The difference between including nsMemory.h and nsCppSharedAllocator.h is only
about 0.05s on my machine (nsMemory is faster).
Assignee | ||
Comment 4•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Whiteboard: [xptest]
Assignee | ||
Comment 5•24 years ago
|
||
bryner says patch #1 and patch #2 build fine in the commercial tree on Linux
Comment 6•24 years ago
|
||
r=bryner on these two patches
Assignee | ||
Comment 7•24 years ago
|
||
bryner tells me that as of today I need to add #include "nsIParser.h" to
nsDocumentViewer.cpp.
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.8
Assignee | ||
Comment 8•24 years ago
|
||
A bunch of stuff in layout/html/content/src/ includes nsIIOService.h and thus a
some other stuff just for NS_MakeAbsoluteURIWithCharset, which uses
nsIIOService::Escape.
Assignee | ||
Comment 9•24 years ago
|
||
Assignee | ||
Comment 10•24 years ago
|
||
A bunch of files include nsScriptSecurityManager.h when they really only need
nsIScriptSecurityManager.h
Assignee | ||
Comment 11•24 years ago
|
||
bryner built this on Windows without additional changes.
Comment 12•24 years ago
|
||
coolness! sr=alecf on all of these
Assignee | ||
Comment 13•24 years ago
|
||
The first round of changes (the above patches) is checked in. I had asked
adamlock to review the nsIWebShell.h change, but he didn't seem to be around
and I didn't want to let this sit around too long, which would either require
more XP testing or bring a greater risk of build bustage.
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.8 → mozilla0.9
Assignee | ||
Comment 14•24 years ago
|
||
FWIW, the patches didn't have a noticeable affect on the tinderbox clobber cycle
times.
Assignee | ||
Comment 15•24 years ago
|
||
Another bad one is that nsIScriptGlobalObject.h includes nsGUIEvent.h.
Assignee | ||
Comment 16•24 years ago
|
||
I have a new patch that does extensive header dependency cleanup.
It only speeds up my build (a non-debug, non-optimized build on a
dual P-733 Linux machine with 256MB RAM) from 32:44 to 31:06.
Assignee | ||
Comment 17•24 years ago
|
||
Assignee | ||
Comment 18•24 years ago
|
||
I'm going to attach a patch that does even more cleanup. It's also against a
more current tree (midday yesterday). The main benefits of this patch aren't in
clobber build times, I guess. It's more likely to improve depend build times in
some cases, and it will also give us the satisfaction of knowing that anything
that includes nsIDocShell.h doesn't have an include dependency on gfx2.
Assignee | ||
Comment 19•24 years ago
|
||
Assignee | ||
Comment 20•24 years ago
|
||
Reality check. Moving out to 0.9.1.
Target Milestone: mozilla0.9 → mozilla0.9.1
Assignee | ||
Comment 21•24 years ago
|
||
So I have something to point to, here's a quick summary of the advantages of
these changes (those that I can think of right now, anyway):
1) They reduce clobber build times by around 5% on my machine (or perhaps more,
since I've made some more reductions since I timed the changes). Reductions for
other machines may vary.
2) They will reduce dependency rebuilds for many changes to headers.
3) They will allow us to better track unwanted module dependencies using
MOZ_TRACK_MODULE_DEPS since we'll be able (using jag's script) to eliminate
many unwanted directories from REQUIRES.
4) We'll have the satisfaction of knowing that including nsIDocShell.h doesn't
bring in gfxtypes.h. :-)
I really am hoping to land this changes sometime, although in reality it
probably won't be until next month (i.e., in 0.9.2). I need to get the approval
of a bunch of module owners and more general approval, and get testing on mac
and windows and on the commercial tree, etc., and I don't have much time right
now. It's not all that hard to keep these changes merged to the tip.
Assignee | ||
Comment 22•24 years ago
|
||
Assignee | ||
Comment 23•24 years ago
|
||
Some notes on things that could be improved in the XPCDOM landing:
* nsIScriptGlobalObject.h now includes jsapi.h
* nsIXPCSecurityManager.h seems to be included all over
* nsIWindowWatcher.h now includes jsapi.h
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Assignee | ||
Comment 24•24 years ago
|
||
Assignee | ||
Comment 25•24 years ago
|
||
From IBMBIDI landing: should remove include of nsIUBidiUtils.h from
nsIPresContext.h.
Assignee | ||
Updated•24 years ago
|
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Comment 26•24 years ago
|
||
as long as it builds, any cleanup like this to mozilla/mailnews is greatly
appreciated.
rs=sspitzer on the mailnews parts.
Assignee | ||
Comment 27•24 years ago
|
||
Assignee | ||
Comment 28•24 years ago
|
||
Assignee | ||
Comment 29•24 years ago
|
||
Assignee | ||
Comment 30•24 years ago
|
||
Assignee | ||
Comment 31•24 years ago
|
||
Netscape commercial tree patches are in
http://bugscape.netscape.com/show_bug.cgi?id=6882
Assignee | ||
Updated•24 years ago
|
Priority: P2 → P1
Assignee | ||
Comment 32•24 years ago
|
||
Comment 33•24 years ago
|
||
r=jag
Comment 34•24 years ago
|
||
rs=brendan@mozilla.org on the whole shebang.
/be
Assignee | ||
Comment 35•24 years ago
|
||
Current pass was checked 2001-07-15 19:40 PDT.
There is further work to be done (for a start, splitting up nsIFrame.h and
reducing includes of what it brings in), so moving to future.
Priority: P1 → P2
Target Milestone: mozilla0.9.3 → Future
Assignee | ||
Comment 36•24 years ago
|
||
Assignee | ||
Comment 37•24 years ago
|
||
I think my preference would be NOT to copy the rcs file for nsIFrame.h, since it
has a long log and the blame for the part I copied out isn't very complex and
one could always refer to nsIFrame.h.
Comment 38•24 years ago
|
||
r/sr=waterson to put nsHTMLReflowState in its own file.
Comment 39•24 years ago
|
||
[s]r=attinasi for patch 43991
Assignee | ||
Updated•24 years ago
|
Target Milestone: Future → mozilla0.9.4
Assignee | ||
Comment 40•24 years ago
|
||
Checked in 2001-07-31 18:27 PDT.
Target Milestone: mozilla0.9.4 → Future
Assignee | ||
Comment 41•23 years ago
|
||
Looks like it might be possible to move nsQuickSort.h from nsVoidArray.h to
nsVoidArray.cpp
Assignee | ||
Comment 42•23 years ago
|
||
rjesup: Do you want to fix nsVoidArray.h, or should I?
Comment 43•23 years ago
|
||
dbaron: I'll take it (and post it here)
Comment 44•23 years ago
|
||
Assignee | ||
Comment 45•23 years ago
|
||
r=dbaron
Assignee | ||
Comment 46•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Priority: P2 → P3
Assignee | ||
Comment 47•21 years ago
|
||
I'm planning to do another pass here now that nsCOMPtr/nsAutoPtr/nsRefPtr don't
require the class to be included. There's also a bit to be gained from
eliminating nsDrawingSurface (a typedef) in favor of |class nsIDrawingSurface|
used in various places. I may also do a patch to try to reduce usage of
nsContentUtils a bit.
Assignee | ||
Comment 48•21 years ago
|
||
Assignee | ||
Comment 49•21 years ago
|
||
Comment on attachment 153353 [details] [diff] [review]
reduce dependencies in content and (a little in) layout
jst, this is only a small piece of what I mentioned earlier (no
nsINodeInfo/nsNodeInfo changes, since they don't help much, alone)
Attachment #153353 -
Flags: superreview?(jst)
Attachment #153353 -
Flags: review?(jst)
Comment 50•21 years ago
|
||
Comment on attachment 153353 [details] [diff] [review]
reduce dependencies in content and (a little in) layout
r+sr=jst, nice to see this cleaned up!
Attachment #153353 -
Flags: superreview?(jst)
Attachment #153353 -
Flags: superreview+
Attachment #153353 -
Flags: review?(jst)
Attachment #153353 -
Flags: review+
Assignee | ||
Comment 51•21 years ago
|
||
This is a prerequisite for some header cleanup.
The bulk of the patch was search-and-replace, but there's a good bit that
wasn't.
Assignee | ||
Updated•21 years ago
|
Attachment #153382 -
Flags: superreview?(roc)
Attachment #153382 -
Flags: review?(roc)
Comment on attachment 153382 [details] [diff] [review]
remove nsDrawingSurface
nice.
Attachment #153382 -
Flags: superreview?(roc)
Attachment #153382 -
Flags: superreview+
Attachment #153382 -
Flags: review?(roc)
Attachment #153382 -
Flags: review+
Assignee | ||
Comment 53•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #153709 -
Flags: superreview?(bzbarsky)
Attachment #153709 -
Flags: review?(bzbarsky)
Comment 54•21 years ago
|
||
Comment on attachment 153709 [details] [diff] [review]
reduce what nsRuleNode.h brings in
r+sr=bzbarsky.
Attachment #153709 -
Flags: superreview?(bzbarsky)
Attachment #153709 -
Flags: superreview+
Attachment #153709 -
Flags: review?(bzbarsky)
Attachment #153709 -
Flags: review+
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 55•17 years ago
|
||
David,
Are you still working on this ?
Assignee | ||
Comment 56•17 years ago
|
||
Not recently.
Product: Mozilla Application Suite → Core
Version: Trunk → unspecified
Updated•15 years ago
|
QA Contact: doronr → general
Assignee | ||
Comment 57•11 years ago
|
||
Let's call this one fixed and use other bugs for newer work.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Summary: header file fan-out slows down compilation → reduce header #include dependencies to speed up compilation
Assignee | ||
Updated•11 years ago
|
Blocks: includehell
You need to log in
before you can comment on or make changes to this bug.
Description
•