Skip to content

Core: Reduce downstream control.h includers#117950

Open
StarryWorm wants to merge 1 commit intogodotengine:masterfrom
StarryWorm:gui-includes-cleanup
Open

Core: Reduce downstream control.h includers#117950
StarryWorm wants to merge 1 commit intogodotengine:masterfrom
StarryWorm:gui-includes-cleanup

Conversation

@StarryWorm
Copy link
Copy Markdown
Contributor

As per Ivorforce's tool, control.h is our 9th most expensive include in terms of compilation.

The issue with this header is that what it includes cannot be reduced, and it is used almost universally in editor classes, thus making it very hard to reduce its impact. This PR aims to reduce the number of files that include it transitively as much as possible.

Unlike #117774, I don't have a clean commit history to present, as this one followed a much messier development process (due to how everything downstream of control.h is tangled together). Splitting the results into commits would most likely prevent the engine from compiling if only commits 1 through n are used (out of N). So the result is one big commit.

An overwhelming majority of the changes are simple includes additions and forward class declarations, however, this PR did make the following "consequential" changes:

  • SpinBoxLineEdit separated to spin_box_line_edit.h from spin_box.h
  • editor_inspector.h & editor_properties.h:
    • EditorProperty moved to editor_properties.h
    • EditorPropertyRevert moved to editor_properties.h
    • EditorInspectorPlugin moved to new editor_inspector_plugin.h
    • EditorInspectorDefaultPlugin moved to new editor_inspector_plugin.h
    • EditorPaginator moved to new editor_paginator.h
    • ArrayPanelContainer moved to new editor_array_panel_container.h
    • EditorInspectorActionButton moved to new editor_inspector_action_button.h

Some method bodies were also moved from their header file to the implementation file to help:

  • node_3d_editor_plugin.h: 6 method bodies moved
  • script_editor_base.h: 1 method body moved
  • animation_player_editor_plugin.h: 2 method bodies moved
  • editor_audio_buses.h: 1 method body moved
  • editor_help.h: 1 method body moved
  • editor_log.h: 6 method bodies moved

Since this PR required quite a lot of analysis to identify what could be reduced, the resulting reports are ~1700 lines long in total. To make it possible for this PR to be editable and load reasonably, you can find the reports in this gist instead:
https://gist.github.com/StarryWorm/f92f3c44f9f1f0b9bacc54fc2c6c1916

Headline stats:

  • control.h includers go from 732 total to 676, a meager 56 (or 7.6%) reduction.
  • Total includes project-wide go from 187709 to 183657, a 4052 (or 2.2%) reduction. This is comprised of:
    • A reduction in transitives from 175349 to 170618, a 4731 (or 2.7%) reduction.
    • An increase in direct includes from 12360 to 13039, a 679 (or 5.5%) increase. This is a good thing, as one long-term goal is that every file include all of the files it needs to include (see IWYU)1
  • 318 files had to be modified for these results, representing 4.5% of all C++ files in the codebase (counting .h and .cpp files only)

The results for control.h are a bit disappointing, and at this point, I believe there isn't much more reduction possible in that number. However, the reduction in codebase-wide transitives is significant, with about 15 transitives removed on average per modified file (this number shoots up if you only look at files with "consequential" changes).

For those who follow these efforts closely: I have learned of quite a few things that can, and will, have automated scripts for. This should make future PRs a lot easier to put together, hopefully helping us achieve our true goal of reducing compile times faster than ever before.

1 with exceptions, primarily regarding exported includes, see IWYU pragmas

@StarryWorm StarryWorm requested review from a team as code owners March 28, 2026 22:42
@Nintorch Nintorch added this to the 4.x milestone Mar 29, 2026
@Nintorch Nintorch removed request for a team March 29, 2026 04:35
@Nintorch Nintorch requested review from a team and removed request for a team March 29, 2026 04:35
Comment thread editor/inspector/editor_properties.h Outdated
Comment thread editor/scene/3d/node_3d_editor_plugin.cpp
Comment thread editor/scene/3d/node_3d_editor_plugin.cpp
Comment thread editor/audio/audio_stream_editor_plugin.h
@StarryWorm StarryWorm force-pushed the gui-includes-cleanup branch from c680446 to c2dd6f5 Compare April 13, 2026 20:35
@StarryWorm StarryWorm force-pushed the gui-includes-cleanup branch from c2dd6f5 to 873c640 Compare April 13, 2026 20:45
@StarryWorm
Copy link
Copy Markdown
Contributor Author

Changelog:

  • Rebased
  • Added ATS' suggestions

@akien-mga
Copy link
Copy Markdown
Member

Great work overall! But seeing the somewhat disappointing results in terms of includers reduction, for a quite significant code churn, I wonder if we're not starting to hit the limits of what we can achieve with forward declares.
Maybe we reached a point where the diminishing returns aren't worth digging deeper in that vein.

WDYT @Ivorforce?

@StarryWorm
Copy link
Copy Markdown
Contributor Author

I think the big thing holding us up in terms of includes is the variant<->object inclusion. If we could decouple those two, I think that would be the big reduction we are looking for. That, however, unless we are willing to do some pretty ugly changes, is locked behind progress on GDType for Variants.

@StarryWorm
Copy link
Copy Markdown
Contributor Author

Regarding how disappointing this change is, I think we need to look at it through the files with meaningful changes, not all the files missing an include or two that needed them added. Yeah, they're still showing churn, but those includes should ideally be there, reduction or not.

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.

4 participants