ello, in the last days, of about 5500 include lines, over 2200 were removed by a simple program that, for every include, tried to compile the source without it.
After a first run, problems were identified and handled by removing overgenerous includes from the most important headers. Also, the problem of non-indented ifdefed includes turned up and needed manual intervention. For this reason, it is possible that compilation in some cases barfs because of overzealous removing.
Another annoying tidbit is the removal of non-indented includes of config.h. The about two dozen cases will be rewritten the next days and should not affect compilation in most cases.
Finally, some includes were removed of the respective header belonging to the source file. This does not affect compilation at all but might annoy some people writing in the source later on.
Please bear with the side effects. I firmly believe that they are worth it all. A log file of the removal is available on request. All in all, a 1.5 GHz computer with 2.5" hard disk needed 12 hours full work for it.
Thanks for your patience, ralf
On Jan 10, 2006, at 4:43 AM, Ralf Stephan wrote:
Please bear with the side effects. I firmly believe that they are worth it all. A log file of the removal is available on request. All in all, a 1.5 GHz computer with 2.5" hard disk needed 12 hours full work for it.
Well... that's not quite the more important aspect. Aside from the CPU spent reducing LOC, are there other metrics you have?
A reduction in build times might be helpful, but an improvement in code quality would probably be much more important. Do you happen to have anything checking standard code metrics before and after?
Also...
One general guideline for improving #includes is to remove #include's from inside of .h files and instead count on including needed ones in the using .cpp files. Among other things, this reduces dependencies, trims hardcoding, improves flexibility and maintainability (with the last one pretty important).
Did you do any passes to address #includes in .h files before removing them from .cpp files? If not, then we can probably just work on that as a next phase.
On Jan 10, 2006, at 4:43 AM, Ralf Stephan wrote:
Please bear with the side effects. I firmly believe that they are worth it all. A log file of the removal is available on request. All in all, a 1.5 GHz computer with 2.5" hard disk needed 12 hours full work for it.
I fixed a couple of compilation problems I noticed. One specifically we may need to be wary of -- removing conditional includes that aren't being included to begin with using whatever preprocessor flags you have while running these scripts.
To explain, one of my fixes was adding back the missing include below:
#ifdef WITH_GNOME_VFS +# include <libgnomevfs/gnome-vfs.h> #endif
I guess this is the danger with automated tools.
Otherwise though, thanks for your time spent on this. Should make compilation times a bit faster.
Cheers, Michael
Did you do any passes to address #includes in .h files before removing them from .cpp files? If not, then we can probably just work on that as a next phase.
Ah thanks for mentioning that. After the orientation run, several hotspots were identified where bunches of specific includes were removed because a .h file contained a general one. Example: .h contains <gtkmm.h>, leads to removal of all specific <gtkmm/...> includes in cpp files.
So, after that, I optimized several heavy duty headers and tried to remove as much general includes from headers as possible. Where this was not possible, only heavy duty ones were so optimized. The situation:
There are no more headers including gtkmm.h, gtk.h There are only isolated headers including glibmm.h There are still a lot of headers including glib.h (someone appeared to have made a sport out of including it in every file) and sigc++.h. I became aware of the latter only after the full run.
So yes, that issue was addressed, though not in full.
ralf
Following myself up:
There are still a lot of headers including glib.h
Now atm, I'd say the remaining ones are merely isolated ones.
... and sigc++.h.
In comparison to glib.h, this could be ignored, as it itself includes only five further files, while with glib the number is 46!
ralf
On Jan 11, 2006, at 12:23 AM, Ralf Stephan wrote:
Did you do any passes to address #includes in .h files before removing them from .cpp files? If not, then we can probably just work on that as a next phase.
Ah thanks for mentioning that. After the orientation run, several hotspots were identified where bunches of specific includes were removed because a .h file contained a general one. Example: .h contains <gtkmm.h>, leads to removal of all specific <gtkmm/...> includes in cpp files.
Actually... that's the opposite of what might be best in many cases.
Removing #includes from the .h in general make each .h file more standalone, easier to substitute things in, and lets the .cpp files drive dependencies.
If it's a standard system include that is needed (e.g. <stdint.h>), its usually safe to assume that the consumer .cpp file will include that once at the top of its includes. It also allows for things like debug macros, wrappers and such to be added in localized situations. Often one can eliminate the need for nested #includes altogether with the judicious use of a few forward declarations instead. (Oh, but not quite the way that src/forward.h used to be)
Then, removing general <gtkmm.h> includes and just using specific <gtkmm/...> includes in .cpp files instead will vastly simplify dependencies and number of files to be included, even though the LOC is higher. On my current system, for example, including <gtkmm.h> immediately does a #include of one hundred and twenty-four other .h files. So by removing four LOC in each .cpp in favor the single shared line in the .h file, you get over a hundred extra includes for each consuming .cpp. Only including the bits you actually need can both simplify maintenance and also speed compile time.
On Jan 11, 2006, at 11:38 PM, Jon A. Cruz wrote:
Actually... that's the opposite of what might be best in many cases.
...
By the way. I want to be very clear that I'm not trying to come down against your effort or anything like that. I believe that #include cleanup is a very good thing to be doing, and that at the end of the day our codebase will end up gaining quite a lot from it.
So be sure to take anything I touch on in this discussion as just my trying to help you refine your understanding and process to improve even them more. (I know things like this can come across as much more harsh than intended when using email, etc.)
So be sure to take anything I touch on in this discussion as just my trying to help you refine your understanding and process to improve even them more. (I know things like this can come across as much more harsh than intended when using email, etc.)
Actually, since I earlier did all those things you tried to convince me of in your previous mail, it only comes across as being a bit behind the action.
ralf
On 1/12/06, Ralf Stephan <ralf@...748...> wrote:
[ snip ]
Actually, since I earlier did all those things you tried to convince me of in your previous mail, it only comes across as being a bit behind the action.
Still further behind...
If necessary, am I O.K. to commit any patch still needed for the issue http://sourceforge.net/tracker/index.php?func=detail&aid=1198588&gro... that is on the Janitors page in the wiki.
I intended to do this about 6 weeks ago, but have had fearful hardware (and other) problems. I don't want to undo any of your good work.
Ben
Ralf Stephan wrote:
For this reason, it is possible that compilation in some cases barfs because of overzealous removing.
Would you be willing to review the changes to prefix.cpp to see if they were in fact all necessary? I believe this file is only used to provide binreloc support for the autopackages.
Aaron Spike
Ralf Stephan wrote:
Would you be willing to review the changes to prefix.cpp to see if they were in fact all necessary? I believe this file is only used to provide binreloc support for the autopackages.
Reverted to previous version.
That seems to allow the autopackage build to compile. Thank you.
Aaron Spike
participants (5)
-
unknown＠example.com -
Ben Fowler -
Jon A. Cruz -
Michael Wybrow -
Ralf Stephan