Updating precompiled files to make building faster.
My computer (8 logic CPUs):
03:19 (02:50 with MP)->01:42 (01:35 with MP) minutes
Details
Details
- Reviewers
vladislavbelov Silier - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22303: Update precompiled headers to improve build times.
- Trac Tickets
- #5038
Rebuild solution without patch. Notice time.
Rebuild solution with patch. Notice time.
Let me know how big there was improvement.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues:
Executing section Default...
Executing section Source...
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'CONFIG2_GLES'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » case·0:
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » case·0:
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » case·0:
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/Handlers/MiscHandlers.cpp
| 33| #include·"ps/Util.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/Handlers/MiscHandlers.cpp
| 33| #include·"ps/Util.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/Handlers/MiscHandlers.cpp
| 33| #include·"ps/Util.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/ps/scripting/JSInterface_Debug.cpp
| 44| » return·*(volatile·int*)0;
| | [MAJOR] CPPCheckBear (nullPointer):
| | Null pointer dereference
Executing section JS...Link to build: https://jenkins.wildfiregames.com/job/differential/107/display/redirect
Comment Actions
Some Profiling
| Without the patch | 2:40 | 3:00 | 3:13 |
| With the patch | 1:46 | 1:42 | 1:55 |
| With the patch and D1262 | 1:46 | 1:43 | 1:42 |
Noticing that the compile time seems steadier when less warnings are output with D1262
Here is the script I used, If you are using vs2017 and the 2015 toolset it should work out of the box (Just to need to edit the cd path)
Else you might have to tweak MSBuild.exe location as well.
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/differential/155/display/redirect
Comment Actions
It doesn't compile for disabled precompiled headers. Also it should be tested for different platforms with/without precompiled headers.
Main notes:
- Empty lines between includes of the same group.
- Deleted headers of a paired file.
- Not sorted includes.
- // in some places.
| source/graphics/Camera.cpp | ||
|---|---|---|
| 29 ↗ | (On Diff #6031) | Empty line, and for others files too. |
| source/graphics/ShaderManager.cpp | ||
| 20 ↗ | (On Diff #6031) | I think it's not good by readability to remove the familiar/main dependent header: ShaderManager.(h|cpp). For other places too. |
| source/pch/graphics/precompiled.h | ||
| 20 ↗ | (On Diff #6031) | Why not sorted? |
| source/ps/SavedGame.cpp | ||
| 27 ↗ | (On Diff #6031) | Useless comment. |
| source/ps/World.cpp | ||
| 42 ↗ | (On Diff #6031) | Useless comment. |
Comment Actions
Removed empty comments, lines (I hope there are not more), reordering includes in precompiled
Patching could be fixed
Cleaned solution, cleaned workspace, updated with --without-pch falg, rebuild solution, result:
Rebuild All: 16 succeeded, 0 failed, 0 skipped
| source/ps/SavedGame.cpp | ||
|---|---|---|
| 27 ↗ | (On Diff #6031) | how is this possible I removed it |
Comment Actions
as a destiny wanted there are some I skipped for somehow
Anyway, I wait untill build and non pch build confirm/failure
| source/ps/Replay.cpp | ||
|---|---|---|
| 27 ↗ | (On Diff #6048) | line |
Comment Actions
Actually you don't disable precompiled headers for Windows, because it defines HAVE_PCH itself, in the lib/sysdep/compiler.h.
Also it doesn't compile on macOS 10.12.6 with the --without-pch param. It seems there will be an error on Linux too.
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues:
Executing section Default...
Executing section Source...
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'CONFIG2_GLES'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/ps/scripting/JSInterface_Debug.cpp
| 42| » return·*(volatile·int*)0;
| | [MAJOR] CPPCheckBear (nullPointer):
| | Null pointer dereference
Executing section JS...Link to build: https://jenkins.wildfiregames.com/job/differential/166/display/redirect
Comment Actions
@Itms, @vladislavbelov
build with HAVE_PCH 0 does not work even without my patch, so I do not think I should care in my patch
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues:
Executing section Source...
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/ActorViewer.cpp
| 33| #include·"graphics/Terrain.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'CONFIG2_GLES'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/View.cpp
| 33| #include·"lib/utf8.h"
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
source/tools/atlas/GameInterface/Misc.cpp
| 1| /*·Copyright·(C)·2009·Wildfire·Games.
| | [NORMAL] LicenseYearBear:
| | License should have "2019" year instead of "2009"
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: ''.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.
source/tools/atlas/GameInterface/Misc.cpp
| 33| » » return·CVector3D(type0.x,·type0.y,·type0.z);
| | [MAJOR] CPPCheckBear (syntaxError):
| | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...
Executing section cli...Link to build: https://jenkins.wildfiregames.com/job/differential/1170/display/redirect
Comment Actions
Measured by time, after a make clean, running make pyrogenesis -o2 -j3
SVN real 5m34.600s user 14m3.430s sys 1m9.786s D1333 real 4m53.224s user 11m34.348s sys 1m3.918s
This means that overall I'm winning about 2.5 minutes, AKA about 40 seconds with multi-thread. It appears logging of the compilation takes a non-trivial amount of time too.
This change seems positive overall, so I'm accepting. Will commit soon-is unless someone else does it first™
Comment Actions
Rm, on second thought, I think we need to keep the original headers in most files. For one thing this would reduce the number of changes (which helps with rebasing old patches and is generally nicer) and for another thing I need to look at what you're actually putting in the precompiled headers. Some of them might increase build times from scratch but cause a bit of pain when modifying these files...
Comment Actions
(I'm commandeering as I didn't want to upload a new revision with a different take on this, @Angen feel free to review this)
After introducing some data into this, through a little helper tool I've made in the past weeks: https://github.com/wraitii/precompiled_helper
My conclusion on what we should do:
Put Clogger and Profile in the core precompiled headers.
Add some more stuff here and there.
With that we cut off several minutes of total compilation time for me and it's not too dangerous.
Should be tested without precompiled headers.
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/pch/simulation2/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/pch/gui/precompiled.h | 1| /*·Copyright·(C)·2010·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2010" source/pch/network/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/atlas/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/engine/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/pch/scriptinterface/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1337/display/redirect
Comment Actions
what about that file in test precompiled header I moved in it was significant improvement. (as I remember)
Comment Actions
Also useful https://stackoverflow.com/questions/1137966/displaying-the-include-hierarchy-for-a-c-file-in-visual-studio This will display all the includes. You can get rid of boost with the regexes
^.*libraries\\win32\\boost\\.*$\n
And of visuals studio like this
^.*C:\\Program Files \(x86\).*$\n
| source/pch/scriptinterface/precompiled.h | ||
|---|---|---|
| 22 ↗ | (On Diff #7923) | These 2 lines are breaking build on VS2015 |
Comment Actions
Add the test thing back, fix VS2015, if tested on windows I'll commit this. We can then work on the precompiled setup cleanup (I think you've got a revision for that too @Angen ).
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Linter detected issues: Executing section Source... source/pch/gui/precompiled.h | 1| /*·Copyright·(C)·2010·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2010" source/pch/atlas/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/network/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/simulation2/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/pch/engine/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/pch/test/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1492/display/redirect
| source/pch/test/precompiled.h | ||
|---|---|---|
| 20 ↗ | (On Diff #8135) | you need to add include guard inside this file |
Comment Actions
Builds without pch on windows (when marked file has include guard). So after that fixed I ll accept it.
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/pch/network/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/engine/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/pch/gui/precompiled.h | 1| /*·Copyright·(C)·2010·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2010" source/pch/test/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/pch/simulation2/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" source/simulation2/system/ComponentTest.h | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/pch/atlas/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1495/display/redirect
Comment Actions
As one can see from jenkins:
Before:
Shell Script -- cd build/workspaces/gcc/ && make -j2 config=debug (self time 8min 55s)
Shell Script -- cd build/workspaces/gcc/ && make -j2 config=release (self time 12min 48s)
After:
Shell Script -- cd build/workspaces/gcc/ && make -j2 config=debug (self time 6min 35s)
Shell Script -- cd build/workspaces/gcc/ && make -j2 config=release (self time 9min 27s)
There's probably room for a bit more improvement in there but it's best to start small.
Comment Actions
Put stuff around HAVE_PCH brackets to respect current conventions, update years. Ready to commit.
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/pch/test/precompiled.h | 1| /*·Copyright·(C)·2009·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2009" source/simulation2/system/ComponentTest.h | 1| /*·Copyright·(C)·2017·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2017" source/pch/simulation2/precompiled.h | 1| /*·Copyright·(C)·2015·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2015" Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1501/display/redirect