Bug 52451

Summary: WebCore should forward declare more often
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: mihaip, sam, tonyg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 32652, 51727, 51730, 52133, 52453, 52522, 52545, 55688, 56296, 60388, 60545    
Bug Blocks:    
Attachments:
Description Flags
SL buildbot compile time none

Tony Gentilcore
Reported 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.
Attachments
SL buildbot compile time (106.93 KB, image/png)
2011-01-17 11:34 PST, Mihai Parparita 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Tony Gentilcore
Comment 1 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.
Sam Weinig
Comment 2 2011-01-15 17:00:07 PST
What patches are you referring to? I also cannot access the second linked spreadsheet.
Mihai Parparita
Comment 3 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)
Tony Gentilcore
Comment 4 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.
Tony Gentilcore
Comment 5 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?
Mihai Parparita
Comment 6 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.
Mihai Parparita
Comment 7 2011-01-17 11:34:54 PST
Created attachment 79196 [details] SL buildbot compile time
Tony Gentilcore
Comment 8 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
Note You need to log in before you can comment on or make changes to this bug.