-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.x] Add support for single compilation unit builds #78113
Conversation
330d443
to
eef9919
Compare
I tested this PR on an Intel Core i9-13900K on Fedora 38:
The gain isn't as pronounced as in the |
Master already has the "lowest hanging fruit" converted, which is most of core, in order to accelerate touching This PR, has quite a bit more in the SCU build than master. It accelerates core like master, but a lot of the added areas are in third party / modules, which isn't wholly recompiled on touching Converting thirdparty for full rebuild hasn't tended to be as dramatic because a lot of it is more difficult to accelerate. There are a lot of small separate libraries (these could potentially be amalgamated), and also global namespace collisions which prevent adding to the SCU build, so only a small fraction is accelerated currently. The option of modifying the code to prevent namespace collisions is also less attractive for third party, as we want to easily be able to update their source code, so we are relying on the library authors to write "clean" code. There is in short a balance to be made for third party between the gains, and any maintenance requirements / cognitive load. We could completely go to town, but if that makes the build hard to maintain, then the costs may exceed the benefits. Timings before / after SCU buildThe overall benefit is quite a complex relationship, and to a certain extent this depends on how efficient the build was before. Master is exceptionally inefficient at compiling (thus more gains are on the table). If you switch on the "show includes" compiler setting for master you can see why - huge dependency chains. The other thing is as you say, on your (super fast) hardware you may be hitting linking bottlenecks quicker than in master. On my slower hardware it's closer to a third of the time for touching |
To clarify, I also have ccache installed, which means a lot of the files from previous builds are reused. This was also the case when benchmarking the |
I think accelerating third-party more is great! However the diff between this PR and the master one I reviewed make it a bit hard to easily approve it without spending a lot of time separating both. It would be easier for me to merge first the equivalent changes to what was done in But that's more work for you to separate things again, so it's not necessarily a good option either. Let me know what you prefer, I can also try to find the time to review this one as is. |
Sure that's not a big deal my end, and it does make sense, easier to review is always good. I'll have a go at this. 👍 |
e929fc8
to
779e02c
Compare
PR is now modified to remove third party changes and be as similar as possible to the master PR. |
|
||
reg_exporters += "\tregister_" + e + "_exporter();\n" | ||
reg_exporters_inc += '#include "platform/' + e + '/export/export.h"\n' | ||
reg_exporters += "}\n" | ||
|
||
# If we are in a SCU build, we add all the platform exporters in one SCU file. | ||
if env["scu_build"]: | ||
env.add_source_files(env.editor_sources, "../platform/*.cpp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be platform/*/export/*.cpp
, no?
This didn't seem needed in the master
version, what's the difference here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight difference here:
In master it compiles each export platform individually. In 3.x, particularly as the export platforms are fairly finalized, these are amalgamated as a single SCU file which is stored in platform/.scu/
folder. (The path is correct, but it will be confusing as this is the first time you have seen this pattern.)
This makes more sense when read with the scu_builders.py
file for this:
process_folder(
[
"platform",
"android/export",
"iphone/export",
"javascript/export",
"osx/export",
"uwp/export",
"windows/export",
"x11/export",
]
)
This is adding all the various platform exporter folders all into a single uber-SCU file which is stored off the platform
folder.
To explain further, the array argument, the first string is the "main" folder, and the following folders are considered sub-folders to be added to the same SCU file.
I'm happy to simplify if preferred to the same method as master, but will lose out on acceleration, but it is worth bearing in mind the exporters are quite significant in the incremental rebuilds. In 3.x, each export directory typically only contains a single file, so there is no accelerating unless they are amalgamated.
...rty/bullet/BulletSoftBody/BulletReducedDeformableBody/btReducedDeformableContactConstraint.h
Outdated
Show resolved
Hide resolved
Adds support for simple SCU build. This speeds up compilation by compiling multiple cpp files within a single translation unit.
Thanks! |
AKA Unity / Jumbo builds.
Adds support for simple SCU build. This speeds up compilation by compiling multiple cpp files within a single translation unit.
This is an optional extra build that occurs when you set
use_scu=yes
in Scons.Supersedes #69405
3.x version of #77170 , see that PR for details.
This PR now removes the third party acceleration so that can be reviewed as a separate PR.
Some benchmarks
(Intel i5-7500T, 2.7ghz x 4, Linux Mint 20, SSD. i.e. low end CPU)
Note: These figures were including third party acceleration, so the PR will now be slightly less efficient.
Full rebuild (DEV)
Normal build 7mins 53
SCU build 3mins 19
Touching
node.h
(DEV)Normal build 4mins 10
SCU build 1min 33
Full rebuild (release)
Normal build 18mins 10
SCU build 12mins 16