Improve compilation time with include-what-you-use (core folder)

[Geometror]

Part of a series of PRs to reduce the compilation time with include-what-you-use together with some manual fixes due to the complexity of the codebase. [see #57734, #57759]

This PR restructures the includes in the core subfolder, so it should probably be merged before the other PRs.

Could help with #53295.

[Geometror]

[copied from RocketChat for record]
Did a bit of a read up, and I think I now have a better understanding of what iwyu does. Now I see that it does it quite well. Although it adds a bunch of includes and removes just a few, it has several benefits:

1. design: better vision/overview of the exact dependencies between files, valuable for refactoring
2. self-containment: even if there are redundant includes, they are basically free because of header guards (and modern compilers) and they can be a good thing since they help to achieve self-containment of headers, making the whole codebase more robust
3. performance: as already said, iwyu does indeed remove some unneeded includes, but only those which can be replaced by a subset of direct includes

For example, if a file xyz.h includes node.h and node.h includes all headers xyz.h would need, but xyz.h also requires some definitions only present in node.h, iwyu will still add all the direct includes to xyz.h and keep the #include "node.h' - in this case there wouldn't be any improvement in build performance, but the benefit of point 1. and 2. still remains: If some includes (which xyz.h originally depended on) are removed from node.h, xyz.h would still compile fine since its self-contained.

Some flaws of iwyu I noticed (all related to templates), which required manual cleanup: Iwyu sometimes thinks a forward declaration is sufficient when it is not, which happens when a type is used as a parameter for a macro using template code. Also specializations of templates distributed across multiple files can cause problems because iwyu does not include all files.

[dmoody256]

Hello recently I did a include what you use tool for scons for mongodb skunkworks. It features a full build analysis mode and a changeset analysis mode: https://github.com/dmoody256/mongo/blob/skunkworks_iwyu/site_scons/site_tools/iwyu.py

I wrote it over the course of a week specifically to work with the mongodb build, so it's definitely alpha level code, but feel free to take ideas from it!

[YuriSizov]

We discussed this among maintainers and agreed that this, as well as #57734 and #57759, should be closed. The spirit of the PR is good, and we definitely would like to have the number of implicit includes reduced, turned into forward declarations, or removed completely where they are just obsolete. Not only would that reduce the build times, it also helps with the codebase health, maintenance, future code improvements, and clarity of scope for each class. But the implementation here, using IWYU, leaves a lot to be desired.

---

This tool cannot be automated because it doesn't understand our codebase well. This means it needs to be run manually from time to time, with manual fixes applied on top of its results. This greatly decreases its helpfulness in terms of maintaining project's codebase health. New PRs will likely introduce new inconsistencies, forget to remove obsolete includes or to propagate their includes to other classes, not directly related to the PR. This will slowly but surely make the generated information from this PR, and subsequent runs of the tool, unreliable.

The output produced by the tool is also very noisy and very verbose. It goes up to the STL level, which is an overkill. It would be great if we could limit it to a reasonable scope, on the project level or even on an engine area level. For example, if we could stop it from going beyond object.h, which is almost guaranteed to be included everywhere, we could significantly reduce the noise-to-signal ratio here. But that doesn't seem to be possible at the moment. Still, even under some ideal circumstances, it would reduce the clarity of the includes section of each class. It will be harder to identify the functionality and the scope of the class if it starts to include everything that it refers to by name.

Even if we could automate the tool, make it run on each commit, and make it reasonably scoped, it will inevitably create a lot of noise to commits themselves. As every changeset modifying the includes will affect every other class directly or indirectly including the current one, propagating the changes throughout the codebase. This will make a lot of PRs touch on areas that they were not designed to touch, making reviews, rebases, and cherry-picks harder. (E.g. changes in GUI can cascade to the editor codebase).

---

All in all, manual work and refactoring seem to be more beneficial and practical. Human developers can identify the underlying issue and rework the code in a meaningful way, adjust includes with a sensibility of a human reader.

[vittorioromeo]

@YuriSizov

All in all, manual work and refactoring seem to be more beneficial and practical. Human developers can identify the underlying issue and rework the code in a meaningful way, adjust includes with a sensibility of a human reader.

I disagree with not merging this PR, as it would have been a better starting point for refactoring compared to what is already there, but I strongly agree with this sentence I quoted.

If you truly believe that, please then let contributors get started on this sort of refactoring with minimal friction. See godotengine/godot-proposals#9016 and #87893.

If I have to spend time arguing about best practices and proving that small refactoring changes do indeed help the codebase compile a bit faster for every single PR I submit, then my motivation to improve the status quo is completely destroyed.

I understand having a high level of scrutiny for larger PRs or for PRs that change the behavior of Godot, but refactoring PRs should feel like a breeze relatively to functionality PRs, otherwise nothing concrete would be achieved in the context of improving the header hygiene and structure of the codebase.

[adamscott]

@YuriSizov

Unfortunately, Yuri announced recently that he wouldn't contribute anymore to the engine.