|Summary:||WebCore should forward declare more often|
|Product:||WebKit||Reporter:||Tony Gentilcore <tonyg>|
|Component:||WebCore Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||mihai, sam, tonyg|
|Version:||528+ (Nightly build)|
|Bug Depends on:||32652, 51727, 51730, 52133, 52453, 52522, 52545, 55688, 56296, 60388, 60545|
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