Closed Bug 64023 Opened 24 years ago Closed 11 years ago

reduce header #include dependencies to speed up compilation

Categories

(Core :: General, defect, P3)

Product:
Core
Component:
General
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: dbaron, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

(Whiteboard: [xptest])

Attachments

(18 files)

patch #1: nsIWebShell.h and nsIDocument.h
8.42 KB, patch
Details | Diff | Splinter Review
patch #2: nsIPref.idl
649 bytes, patch
Details | Diff | Splinter Review
additional changes needed to build on mac (from sfraser)
1.92 KB, patch
Details | Diff | Splinter Review
header include dependency cleanup
148.59 KB, patch
Details | Diff | Splinter Review
patch against April 15 tree
159.53 KB, patch
Details | Diff | Splinter Review
patch against May 7 tree
173.56 KB, patch
Details | Diff | Splinter Review
against May 9, 4AM tree (after XPCDOM landing)
182.84 KB, patch
Details | Diff | Splinter Review
Patch against June 20, 8AM tree
269.59 KB, patch
Details | Diff | Splinter Review
same thing, but also compiles on Windows and with DEBUG
198.95 KB, patch
Details | Diff | Splinter Review
also builds on Mac (patch excludes new file, nsEvent.h)
198.75 KB, patch
Details | Diff | Splinter Review
builds on Linux, Windows, and Mac
213.08 KB, patch
Details | Diff | Splinter Review
against 2001-07-04 tree, updated from previous and built on Windows
214.13 KB, patch
Details | Diff | Splinter Review
patch to split nsHTMLReflowMetrics out of nsIFrame.h
14.03 KB, patch
Details | Diff | Splinter Review
remove nsQuickSort.h from nsVoidArray.h
875 bytes, patch
Details | Diff | Splinter Review
the beginnings of another pass (compiles on Linux opt build)
24.20 KB, patch
Details | Diff | Splinter Review
reduce dependencies in content and (a little in) layout
13.16 KB, patch
jst
: review+
jst
: superreview+
Details | Diff | Splinter Review
remove nsDrawingSurface
230.84 KB, patch
roc
: review+
roc
: superreview+
Details | Diff | Splinter Review
reduce what nsRuleNode.h brings in
47.86 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
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
Actually, lots of very common header files #include <stdio.h>.
The difference between including nsMemory.h and nsCppSharedAllocator.h is only
about 0.05s on my machine (nsMemory is faster).
Whiteboard: [xptest]
bryner says patch #1 and patch #2 build fine in the commercial tree on Linux
r=bryner on these two patches
bryner tells me that as of today I need to add #include "nsIParser.h" to
nsDocumentViewer.cpp.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.8
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.
A bunch of files include nsScriptSecurityManager.h when they really only need
nsIScriptSecurityManager.h
bryner built this on Windows without additional changes.
coolness! sr=alecf on all of these
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.
Target Milestone: mozilla0.8 → mozilla0.9
FWIW, the patches didn't have a noticeable affect on the tinderbox clobber cycle
times.
Another bad one is that nsIScriptGlobalObject.h includes nsGUIEvent.h.
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.
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.
Reality check.  Moving out to 0.9.1.
Target Milestone: mozilla0.9 → mozilla0.9.1
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.
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
Target Milestone: mozilla0.9.1 → mozilla0.9.2
From IBMBIDI landing:  should remove include of nsIUBidiUtils.h from
nsIPresContext.h.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
as long as it builds, any cleanup like this to mozilla/mailnews is greatly
appreciated.

rs=sspitzer on the mailnews parts.
r=jag
rs=brendan@mozilla.org on the whole shebang.

/be
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
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.
r/sr=waterson to put nsHTMLReflowState in its own file.
[s]r=attinasi for patch 43991
Target Milestone: Future → mozilla0.9.4
Checked in 2001-07-31 18:27 PDT.
Target Milestone: mozilla0.9.4 → Future
Looks like it might be possible to move nsQuickSort.h from nsVoidArray.h to
nsVoidArray.cpp
rjesup: Do you want to fix nsVoidArray.h, or should I?
dbaron: I'll take it (and post it here)

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.
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 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+
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.
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+
Attachment #153709 - Flags: superreview?(bzbarsky)
Attachment #153709 - Flags: review?(bzbarsky)
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+
Depends on: 254790
Product: Browser → Seamonkey
David,
Are you still working on this ?
Not recently.
Product: Mozilla Application Suite → Core
Version: Trunk → unspecified
QA Contact: doronr → general
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: