Bug 52451 - WebCore should forward declare more often
Summary: WebCore should forward declare more often
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 32652 51727 51730 52133 52453 52522 52545 55688 56296 60388 60545
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-14 08:45 PST by Tony Gentilcore
Modified: 2011-05-10 05:46 PDT (History)
3 users (show)

See Also:


Attachments
SL buildbot compile time (106.93 KB, image/png)
2011-01-17 11:34 PST, Mihai Parparita
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Gentilcore 2011-01-14 08:45:59 PST
In WebCore there are a lot of instances of one header including another when it could forward declare instead. Using forward declares more aggressively can improve compile time, especially incremental.

The Visual Studio IncludeManager plugin generated this list containing the number of includers/includees for each file as well as the number of tokens and an estimation of their build impact.
https://spreadsheets0.google.com/ccc?key=tZzn5fgBhBIzWyqm4KpDJaQ#gid=0

I've simply been going through some of the more expensive headers and looking at each include line to see if it can be forward declared instead. I encourage others to pick a few, it is a surprisingly zen task.
Comment 1 Tony Gentilcore 2011-01-15 10:26:07 PST
I ran a script overnight on my laptop (OSX 10.6.4, 2.5GHz Core 2 Duo, 4G, XCode 3.2.5) which does a clean build before and after each of these forward declaration patches (an initial warm-up build was thrown away).

While completely unscientific (only one build at each), it suggests each of these patches trimmed 3-5% from the build time. Accounting for regression in between the patches, the build is about 7% faster after the 4th patch than before the first.

Details: https://spreadsheets.google.com/ccc?key=taK4Vvkts8H1m7dnbeg6Hjw

IMO, it seems to be worthwhile to continue forward declaration patches.
Comment 2 Sam Weinig 2011-01-15 17:00:07 PST
What patches are you referring to?  I also cannot access the second linked spreadsheet.
Comment 3 Mihai Parparita 2011-01-15 17:47:16 PST
I believe Tony is referring to changes like http://trac.webkit.org/changeset/75837 and http://trac.webkit.org/changeset/75377.

Since these changes have been rolled upstream to Chromium, I was hoping they would have an effect on the cycle time of the Chromium WebKit builders, but I'm not seeing much effect (albeit the graphs are noisy and have annoying scales):

http://build.chromium.org/p/chromium/stats/Webkit%20Win%20Builder%20(dbg)
http://build.chromium.org/p/chromium/stats/Webkit%20Mac%20Builder
http://build.chromium.org/p/chromium/stats/Webkit%20Linux
http://build.chromium.org/p/chromium/stats/Webkit%20Linux%20Builder%20(dbg)
Comment 4 Tony Gentilcore 2011-01-15 18:21:04 PST
(In reply to comment #2)
> What patches are you referring to?
Yes, the ones this bug depends on.

> I also cannot access the second linked spreadsheet.
Oops. Permissions are fixed now.
Comment 5 Tony Gentilcore 2011-01-15 18:26:42 PST
> Since these changes have been rolled upstream to Chromium, I was hoping they would have an effect on the cycle time of the Chromium WebKit builders, but I'm not seeing much effect (albeit the graphs are noisy and have annoying scales):

There's no label on the horizontal axis of those graphs. Do you have any idea what sort of timescale they represent?
Comment 6 Mihai Parparita 2011-01-17 11:34:23 PST
(In reply to comment #5)
> There's no label on the horizontal axis of those graphs. Do you have any idea what sort of timescale they represent?

No idea, but seconds perhaps?

I messed around with the build.webkit.org JSON output to make http://persistent.info/webkit/tools/buildbot/

You can choose the Snow Leopard builder and show the last 200 builds and highlight "tonyg" to see when your changes landed. Unfortunately no easily discernable effect (even with smoothing, controlled by the box in the lower left). I'm guessing the builds are too different (as far as what files they touch), so perhaps buildbots are not the best place to measure the impact of this.
Comment 7 Mihai Parparita 2011-01-17 11:34:54 PST
Created attachment 79196 [details]
SL buildbot compile time
Comment 8 Tony Gentilcore 2011-03-08 09:22:58 PST
The include-what-you-use project looks like it has the potential to solve this once and for all.

There are chromium-port specific instructions for running it here:
http://code.google.com/p/chromium/wiki/IncludeWhatYouUse

But, there are two (related) issues which prevent it from being really useful in WebKit:
http://code.google.com/p/include-what-you-use/issues/detail?id=5
http://code.google.com/p/include-what-you-use/issues/detail?id=16