Skip to content
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

Core: Avoid including modules/modules_enabled.gen.h in headers #100023

Merged

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Dec 4, 2024

Because modules/modules_enabled.gen.h regenerates anytime a module is changed, this causes all files with this header to necessitate a rebuild. For definition files this is generally fine, but header files can cause cascading recompilations across dozens, if not hundreds, of unnecessary files. Taking the test format from #99302, the refactors from this PR resulted in:

Results
scons: Building targets ...
[  4%] Generating modules\modules_enabled.gen.h ...
[ 79%] Compiling editor\editor_node.cpp ...
[ 79%] Generating modules\register_module_types.gen.cpp ...
[ 79%] Generating modules\modules_tests.gen.h ...
[ 79%] Compiling modules\register_module_types.gen.cpp ...
[ 79%] Compiling tests\test_main.cpp ...
[ 79%] Compiling modules\navigation\3d\nav_mesh_generator_3d.cpp ...
[ 79%] Compiling editor\scene_tree_dock.cpp ...
[ 79%] Compiling modules\gdscript\language_server\gdscript_workspace.cpp ...
[ 79%] Compiling editor\themes\editor_icons.cpp ...
[ 79%] Compiling platform\windows\export\export_plugin.cpp ...
[ 79%] Compiling modules\gdscript\language_server\gdscript_language_protocol.cpp ...
[ 79%] Compiling scene\theme\default_theme.cpp ...
[ 79%] Compiling platform\macos\export\export_plugin.cpp ...
[ 79%] Compiling scene\gui\rich_text_label.cpp ...
[ 79%] Compiling platform\web\export\export_plugin.cpp ...
[ 79%] Compiling modules\text_server_fb\register_types.cpp ...
[ 79%] Compiling modules\text_server_adv\thorvg_svg_in_ot.cpp ...
[ 79%] Compiling editor\export\codesign.cpp ...
[ 79%] Compiling editor\plugins\bone_map_editor_plugin.cpp ...
[ 79%] Compiling modules\gdscript\language_server\gdscript_language_server.cpp ...
[ 79%] Compiling platform\ios\export\export_plugin.cpp ...
[ 79%] Compiling modules\gdscript\language_server\gdscript_extend_parser.cpp ...
[ 79%] Compiling modules\navigation\3d\godot_navigation_server_3d.cpp ...
[ 79%] Compiling editor\project_converter_3_to_4.cpp ...
[ 79%] Compiling main\main.cpp ...
[ 79%] Compiling scene\resources\shader.cpp ...
[ 79%] Compiling platform\android\export\export_plugin.cpp ...
[ 79%] Compiling editor\rename_dialog.cpp ...
[ 79%] Compiling editor\editor_help.cpp ...
[ 79%] Compiling modules\gdscript\language_server\gdscript_text_document.cpp ...
[ 79%] Compiling editor\doc_tools.cpp ...
[ 80%] Compiling platform\linuxbsd\export\export_plugin.cpp ...
[ 80%] Compiling modules\gltf\gltf_document.cpp ...
[ 80%] Linking Static Library modules\modules.windows.editor.dev.x86_64.lib ...
[ 80%] Compiling modules\text_server_adv\register_types.cpp ...
[ 80%] Compiling editor\import\resource_importer_dynamic_font.cpp ...
[ 80%] Compiling modules\text_server_adv\text_server_adv.cpp ...
[ 80%] Compiling editor\register_editor_types.cpp ...
[ 80%] Compiling core\io\logger.cpp ...
[ 80%] Compiling modules\text_server_fb\text_server_fb.cpp ...
[ 80%] Compiling modules\gdscript\register_types.cpp ...
[ 80%] Compiling core\config\project_settings.cpp ...
[ 80%] Compiling modules\text_server_fb\thorvg_svg_in_ot.cpp ...
[ 80%] Linking Static Library modules\module_navigation.windows.editor.dev.x86_64.lib ...
[ 81%] Linking Static Library scene\scene.windows.editor.dev.x86_64.lib ...
[ 85%] Linking Static Library core\core.windows.editor.dev.x86_64.lib ...
[ 85%] Linking Static Library modules\module_gltf.windows.editor.dev.x86_64.lib ...
[ 88%] Linking Static Library main\main.windows.editor.dev.x86_64.lib ...
[ 88%] Linking Static Library modules\module_text_server_fb.windows.editor.dev.x86_64.lib ...
[ 89%] Linking Static Library modules\module_gdscript.windows.editor.dev.x86_64.lib ...
[ 92%] Linking Static Library modules\module_text_server_adv.windows.editor.dev.x86_64.lib ...
[ 97%] Building compilation database compile_commands.json
[ 99%] Linking Static Library editor\editor.windows.editor.dev.x86_64.lib ...
[100%] Building node count database .scons_node_count
[100%] Linking Static Library tests\tests.windows.editor.dev.x86_64.lib ...
[100%] Linking Program bin\godot.windows.editor.dev.x86_64.exe ...
[100%] Linking Program bin\godot.windows.editor.dev.x86_64.console.exe ...
[100%] scons: done building targets.
[Time elapsed: 00:01:00.56]
So while this doesn't reach the peak speed of #99302's implementation, those speeds were only possible by fundamentally changing how modules were parsed. Not only does this PR leave the module logic itself entirely untouched, but it does so while improving compile time threefold!

@Repiteo Repiteo added this to the 4.4 milestone Dec 4, 2024
@Repiteo Repiteo requested review from a team as code owners December 4, 2024 20:59
@Repiteo Repiteo force-pushed the core/module-includes-header-strip branch from 610fffe to ab67255 Compare January 7, 2025 23:28
@Repiteo Repiteo requested a review from a team as a code owner January 7, 2025 23:28
@Repiteo Repiteo force-pushed the core/module-includes-header-strip branch from 9916f61 to 4848dac Compare January 9, 2025 03:57
@Repiteo Repiteo requested review from a team as code owners January 9, 2025 03:57
@Repiteo Repiteo force-pushed the core/module-includes-header-strip branch from 4848dac to 1a76167 Compare January 9, 2025 03:59
@Repiteo Repiteo removed request for a team January 9, 2025 04:03
@Repiteo Repiteo force-pushed the core/module-includes-header-strip branch from 1a76167 to ec190ba Compare January 24, 2025 22:17
@Repiteo Repiteo modified the milestones: 4.4, 4.5 Jan 24, 2025
@Repiteo Repiteo removed request for a team January 24, 2025 22:17
@Repiteo Repiteo requested review from a team and removed request for a team January 24, 2025 22:18
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@Repiteo Repiteo force-pushed the core/module-includes-header-strip branch from ec190ba to 6c4f17d Compare March 12, 2025 00:45
@Repiteo Repiteo merged commit 0a30831 into godotengine:master Mar 12, 2025
20 checks passed
@Repiteo Repiteo deleted the core/module-includes-header-strip branch March 12, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants