[chromium-dev] Re: Don't include render_messages.h in a header
I fear we have a have a couple long threads of headers that touch everything. At one point we had something like: v8 bindings -> stats table -> process_util -> process, which meant if you touched any of our process-management code we'd rebuild all of WebKit's SVG bindings. :~( For an especially painful build, try touching npapi.h. Apparently most of our project depends on that file. On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek wrote: > > I've gone through the code and removed all such occurrences. This > speeds up the build from 15 to 13 minutes (using /MP on quadcore with > SSD). It also means that editing that file only rebuilds 36 files > instead of 200. > > There shouldn't be any reason to include that file from a header. The > reason this has happened is that people have ended up using structs > that define the parameters of an IPC message with many parameters as > member variables in classes. If this happens, then the struct should > just be moved to its own file outside of render_messages.h. > > > > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: > For an especially painful build, try touching npapi.h. Apparently > most of our project depends on that file. Yeah, I made that discovery last night. Very painful. --Amanda --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
One idea I've been thinking about is timing how long a clean build takes, and tracking that just as we do with other performance tests. The goal would be to notice when someone introduces an unnecessary dependency that slows the build. Obviously if it's needed, that's ok. Chromium-XP does a full build each time, I wonder how easy it would be to plot its build times? On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: > I fear we have a have a couple long threads of headers that touch > everything. At one point we had something like: v8 bindings -> stats > table -> process_util -> process, which meant if you touched any of > our process-management code we'd rebuild all of WebKit's SVG bindings. > :~( > > For an especially painful build, try touching npapi.h. Apparently > most of our project depends on that file. > > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek wrote: >> >> I've gone through the code and removed all such occurrences. This >> speeds up the build from 15 to 13 minutes (using /MP on quadcore with >> SSD). It also means that editing that file only rebuilds 36 files >> instead of 200. >> >> There shouldn't be any reason to include that file from a header. The >> reason this has happened is that people have ended up using structs >> that define the parameters of an IPC message with many parameters as >> member variables in classes. If this happens, then the struct should >> just be moved to its own file outside of render_messages.h. >> >> >> >> > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
(arg. reposting again) One thing that scons is very good at is dependency analysis. Is it possible to get it to spit out a dependency report (which files depend on each file in the project)? I wonder if we might find that files like npapi.h don't actually need to touch everything, etc. Erik On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: > > I fear we have a have a couple long threads of headers that touch > everything. At one point we had something like: v8 bindings -> stats > table -> process_util -> process, which meant if you touched any of > our process-management code we'd rebuild all of WebKit's SVG bindings. > :~( > > For an especially painful build, try touching npapi.h. Apparently > most of our project depends on that file. > > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek wrote: >> >> I've gone through the code and removed all such occurrences. This >> speeds up the build from 15 to 13 minutes (using /MP on quadcore with >> SSD). It also means that editing that file only rebuilds 36 files >> instead of 200. >> >> There shouldn't be any reason to include that file from a header. The >> reason this has happened is that people have ended up using structs >> that define the parameters of an IPC message with many parameters as >> member variables in classes. If this happens, then the struct should >> just be moved to its own file outside of render_messages.h. >> >> > >> > > > > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
That shouldn't be a problem anymore since WebKit no longer depends directly on base/. -Darin On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: > > I fear we have a have a couple long threads of headers that touch > everything. At one point we had something like: v8 bindings -> stats > table -> process_util -> process, which meant if you touched any of > our process-management code we'd rebuild all of WebKit's SVG bindings. > :~( > > For an especially painful build, try touching npapi.h. Apparently > most of our project depends on that file. > > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek > wrote: > > > > I've gone through the code and removed all such occurrences. This > > speeds up the build from 15 to 13 minutes (using /MP on quadcore with > > SSD). It also means that editing that file only rebuilds 36 files > > instead of 200. > > > > There shouldn't be any reason to include that file from a header. The > > reason this has happened is that people have ended up using structs > > that define the parameters of an IPC message with many parameters as > > member variables in classes. If this happens, then the struct should > > just be moved to its own file outside of render_messages.h. > > > > > > > > > > > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
Hi Erik-- It depends on what you really want. SCons does have all of the dependencies, but it doesn't keep the #include tree. Tangible example: if "foo.c" #includes "inc.h", and "inc.h" also #includes both "a.h" and "b.h", the resulting dependency list actually looks like: foo.o: foo.c inc.h a.h b.h It's trivial to dump that sort of information from SCons' database (there's a "sconsign" script that will do it). It would not show you which files #included each other, which for this example would look something like: foo.o: foo.c foo.c: inc.h inc.h: a.h b.h (SCons doesn't track those #include relationships are not actually dependencies.) Now that I think about it, I'm not sure how you'd use either set of information to determine automatically that a dependency isn't needed. The dependencies only exist because there's actually a #include line somewhere in the chain that will (or might) suck it into the compile. Trying to figure out if a given #include is somehow unnecessary for a given .o file seems like a garbage-in-garbage-out problem; I don't see where you'd derive the distinction that indicates one #include is necessary but another isn't. But if someone does come up with a use for the dependency info (per the first example above), it's easy enough to get. --SK On Thu, Feb 5, 2009 at 11:26 AM, Erik Kay wrote: > > (arg. reposting again) > > One thing that scons is very good at is dependency analysis. Is it > possible to get it to spit out a dependency report (which files depend > on each file in the project)? I wonder if we might find that files > like npapi.h don't actually need to touch everything, etc. > > Erik > > > On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: > > > > I fear we have a have a couple long threads of headers that touch > > everything. At one point we had something like: v8 bindings -> stats > > table -> process_util -> process, which meant if you touched any of > > our process-management code we'd rebuild all of WebKit's SVG bindings. > > :~( > > > > For an especially painful build, try touching npapi.h. Apparently > > most of our project depends on that file. > > > > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek > wrote: > >> > >> I've gone through the code and removed all such occurrences. This > >> speeds up the build from 15 to 13 minutes (using /MP on quadcore with > >> SSD). It also means that editing that file only rebuilds 36 files > >> instead of 200. > >> > >> There shouldn't be any reason to include that file from a header. The > >> reason this has happened is that people have ended up using structs > >> that define the parameters of an IPC message with many parameters as > >> member variables in classes. If this happens, then the struct should > >> just be moved to its own file outside of render_messages.h. > >> > >> > > >> > > > > > > > > > > > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
(sorry for repost, gmail sucks and I have a short memory) On Thu, Feb 5, 2009 at 2:51 PM, Steven Knight wrote: > Hi Erik-- > It depends on what you really want. SCons does have all of the > dependencies, but it doesn't keep the #include tree. > Tangible example: if "foo.c" #includes "inc.h", and "inc.h" also #includes > both "a.h" and "b.h", the resulting dependency list actually looks like: > foo.o: foo.c inc.h a.h b.h > > It's trivial to dump that sort of information from SCons' database (there's > a "sconsign" script that will do it). > It would not show you which files #included each other, which for this > example would look something like: > foo.o: foo.c > foo.c: inc.h > inc.h: a.h b.h > (SCons doesn't track those #include relationships are not actually > dependencies.) Right, I figured all of the info was there. I guess the question was whether it was easy to do something like this: $ scons_deps a.h foo.c $ scons_deps -v a.h foo.c:inc.h:a.h $ scons_deps -v foo.c:inc.h:a.h foo.c:inc.h:b.h bar.c:bar.h etc. Does such a script (or something similar) exist? From what you're saying, I'm assuming it's easy to write. > Now that I think about it, I'm not sure how you'd use either set of > information to determine automatically that a dependency isn't needed. The > dependencies only exist because there's actually a #include line somewhere > in the chain that will (or might) suck it into the compile. Trying to > figure out if a given #include is somehow unnecessary for a given .o file > seems like a garbage-in-garbage-out problem; I don't see where you'd derive > the distinction that indicates one #include is necessary but another isn't. > But if someone does come up with a use for the dependency info (per the > first example above), it's easy enough to get. Right, I wasn't assuming you'd be able to figure out if a given dependency was necessary in an automated way. The point of this would be to emit a report that then could be examined by a human. We could easily munge the data and answer questions like "which header is most depended on in the source?", or "which header file adds the most bytes to a build?" (size in bytes * number of files included) Erik > On Thu, Feb 5, 2009 at 11:26 AM, Erik Kay wrote: >> >> (arg. reposting again) >> >> One thing that scons is very good at is dependency analysis. Is it >> possible to get it to spit out a dependency report (which files depend >> on each file in the project)? I wonder if we might find that files >> like npapi.h don't actually need to touch everything, etc. >> >> Erik >> >> >> On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: >> > >> > I fear we have a have a couple long threads of headers that touch >> > everything. At one point we had something like: v8 bindings -> stats >> > table -> process_util -> process, which meant if you touched any of >> > our process-management code we'd rebuild all of WebKit's SVG bindings. >> > :~( >> > >> > For an especially painful build, try touching npapi.h. Apparently >> > most of our project depends on that file. >> > >> > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek >> > wrote: >> >> >> >> I've gone through the code and removed all such occurrences. This >> >> speeds up the build from 15 to 13 minutes (using /MP on quadcore with >> >> SSD). It also means that editing that file only rebuilds 36 files >> >> instead of 200. >> >> >> >> There shouldn't be any reason to include that file from a header. The >> >> reason this has happened is that people have ended up using structs >> >> that define the parameters of an IPC message with many parameters as >> >> member variables in classes. If this happens, then the struct should >> >> just be moved to its own file outside of render_messages.h. >> >> >> >> > >> >> >> > >> > > >> > >> >> >> > > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 5, 2009 at 4:02 PM, Erik Kay wrote: > $ scons_deps -v > foo.c:inc.h:a.h > foo.c:inc.h:b.h > bar.c:bar.h > etc. > > Does such a script (or something similar) exist? From what you're > saying, I'm assuming it's easy to write. Note that you could always tell MSVC to turn on printing #include chains for each compiled file, then compile the various files you're interested in. From this you could probably create a reverse-mapping if you wanted... PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 05, 2009 at 04:04:58PM -0800, Peter Kasting wrote: > On Thu, Feb 5, 2009 at 4:02 PM, Erik Kay wrote: > > > $ scons_deps -v > > foo.c:inc.h:a.h > > foo.c:inc.h:b.h > > bar.c:bar.h > > etc. > > > > Does such a script (or something similar) exist? From what you're > > saying, I'm assuming it's easy to write. > > > Note that you could always tell MSVC to turn on printing #include chains for > each compiled file, then compile the various files you're interested in. > From this you could probably create a reverse-mapping if you wanted... The same can be done with gcc/g++ by passing the -H argument when compiling. -- James GPG Key: 1024D/61326D40 2003-09-02 James Vega --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 5, 2009 at 4:21 PM, James Vega wrote: > > On Thu, Feb 05, 2009 at 04:04:58PM -0800, Peter Kasting wrote: >> On Thu, Feb 5, 2009 at 4:02 PM, Erik Kay wrote: >> >> > $ scons_deps -v >> > foo.c:inc.h:a.h >> > foo.c:inc.h:b.h >> > bar.c:bar.h >> > etc. >> > >> > Does such a script (or something similar) exist? From what you're >> > saying, I'm assuming it's easy to write. >> >> >> Note that you could always tell MSVC to turn on printing #include chains for >> each compiled file, then compile the various files you're interested in. >> From this you could probably create a reverse-mapping if you wanted... > > The same can be done with gcc/g++ by passing the -H argument when > compiling. I did the same with -MM and parsed the results. Assuming I did it right, the average .cc file in src/chrome/ includes about 135 header files. The files with the most number of includes: 440 about_chrome_view.o 470 options_window_view.o 481 history_ui.o 483 user_data_dir_dialog.o 544 dns_host_info_unittest.o 601 aero_glass_non_client_view.o --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 5, 2009 at 4:21 PM, James Vega wrote: > > On Thu, Feb 05, 2009 at 04:04:58PM -0800, Peter Kasting wrote: >> On Thu, Feb 5, 2009 at 4:02 PM, Erik Kay wrote: >> >> > $ scons_deps -v >> > foo.c:inc.h:a.h >> > foo.c:inc.h:b.h >> > bar.c:bar.h >> > etc. >> > >> > Does such a script (or something similar) exist? From what you're >> > saying, I'm assuming it's easy to write. >> >> >> Note that you could always tell MSVC to turn on printing #include chains for >> each compiled file, then compile the various files you're interested in. >> From this you could probably create a reverse-mapping if you wanted... > > The same can be done with gcc/g++ by passing the -H argument when > compiling. I did the same with -MM and parsed the results. Assuming I did it right, the average .cc file in src/chrome/ includes about 135 header files. The files with the most number of includes: 440 about_chrome_view.o 470 options_window_view.o 481 history_ui.o 483 user_data_dir_dialog.o 544 dns_host_info_unittest.o 601 aero_glass_non_client_view.o --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 5, 2009 at 4:35 PM, Lei Zhang wrote: > I did the same with -MM and parsed the results. Assuming I did it > right, the average .cc file in src/chrome/ includes about 135 header > files. The files with the most number of includes: > > 440 about_chrome_view.o > 470 options_window_view.o > 481 history_ui.o > 483 user_data_dir_dialog.o > 544 dns_host_info_unittest.o > 601 aero_glass_non_client_view.o What we really need is some sort of graph of includes, so we could find long chains. From this list I don't know which headers are most at fault. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Thu, Feb 05, 2009 at 04:49:34PM -0800, Evan Martin wrote: > > On Thu, Feb 5, 2009 at 4:35 PM, Lei Zhang wrote: > > I did the same with -MM and parsed the results. Assuming I did it > > right, the average .cc file in src/chrome/ includes about 135 header > > files. The files with the most number of includes: > > > > 440 about_chrome_view.o > > 470 options_window_view.o > > 481 history_ui.o > > 483 user_data_dir_dialog.o > > 544 dns_host_info_unittest.o > > 601 aero_glass_non_client_view.o > > What we really need is some sort of graph of includes, so we could > find long chains. From this list I don't know which headers are most > at fault. The raw output gives that sort of display Compiling Hammer/dbg/obj/chrome/app/chrome_exe_main_gtk.o . /home/jamessan/src/chromium/src/base/at_exit.h .. /usr/include/c++/4.3/stack ... /usr/include/c++/4.3/deque /usr/include/c++/4.3/bits/stl_algobase.h . /usr/include/c++/4.3/i486-linux-gnu/bits/c++config.h .. /usr/include/c++/4.3/i486-linux-gnu/bits/os_defines.h ... /usr/include/features.h /usr/include/sys/cdefs.h . /usr/include/bits/wordsize.h /usr/include/gnu/stubs.h . /usr/include/bits/wordsize.h . /usr/include/gnu/stubs-32.h .. /usr/include/c++/4.3/i486-linux-gnu/bits/cpu_defines.h . /usr/include/c++/4.3/cstddef .. /usr/lib/gcc/i486-linux-gnu/4.3.2/include/stddef.h so it shouldn't be too bad to generate such information. -- James GPG Key: 1024D/61326D40 2003-09-02 James Vega --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Don't include render_messages.h in a header
On Fri, Feb 6, 2009 at 10:54 AM, Nicolas Sylvain wrote: > > > On Thu, Feb 5, 2009 at 10:56 AM, John Abd-El-Malek wrote: > >> >> One idea I've been thinking about is timing how long a clean build >> takes, and tracking that just as we do with other performance tests. >> The goal would be to notice when someone introduces an unnecessary >> dependency that slows the build. Obviously if it's needed, that's ok. >> >> Chromium-XP does a full build each time, I wonder how easy it would be >> to plot its build times? > > > I attached a graph of the build time of Chromium XP over the last 3000 > changelists. > > I manually removed all the really high values (10% higher than the previous > and the next build), otherwise it was too noisy. > > It's still really noisy, but we can see an increase of about 1 minute and > 40 seconds to the compile time between revision 7755 and 7774. > > It was not obvious to me what was the cause by looking at all the changes > in this range. Maybe a webkit merge. Maybe the new dependency on browser. > > It's important to note that this machine is using incredibuild. > Interesting graph. I think using the time from incredibuild will be noisy due to how many other builds are going on at the same time. What would be more work, but more accurate, would be if we kick off a full build on the other machines without incredibuild (or this one with IB disabled) every night. > > Nicolas > > > >> >> On Thu, Feb 5, 2009 at 10:48 AM, Evan Martin wrote: >> > I fear we have a have a couple long threads of headers that touch >> > everything. At one point we had something like: v8 bindings -> stats >> > table -> process_util -> process, which meant if you touched any of >> > our process-management code we'd rebuild all of WebKit's SVG bindings. >> > :~( >> > >> > For an especially painful build, try touching npapi.h. Apparently >> > most of our project depends on that file. >> > >> > On Thu, Feb 5, 2009 at 10:29 AM, John Abd-El-Malek >> wrote: >> >> >> >> I've gone through the code and removed all such occurrences. This >> >> speeds up the build from 15 to 13 minutes (using /MP on quadcore with >> >> SSD). It also means that editing that file only rebuilds 36 files >> >> instead of 200. >> >> >> >> There shouldn't be any reason to include that file from a header. The >> >> reason this has happened is that people have ended up using structs >> >> that define the parameters of an IPC message with many parameters as >> >> member variables in classes. If this happens, then the struct should >> >> just be moved to its own file outside of render_messages.h. >> >> >> >> >> >> >> >> > >> >> >> >> > --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
