Hyprland
Hyprland copied to clipboard
hyprwm
refactor: only include what you use
Currently, header usage in Hyprland is rather chaotic, some headers have circular dependencies, some headers or source includes have more than they actually need.
This can cause unexpected compiler/linker errors and increase maintenance costs. It also slows down PCH disabled and incremental builds. Since unnecessary files are included, the build system will recompile where it's not necessary.
Another problem is with the development tools, especially the language servers such as clangd, where it reports mysterious errors in headers included in headers included in headers included in headers included in headers...
Thus this PR propose that each source file should contain only and directly what it uses.
Advantages
Reduces incremental build time
For example, suppose we do an initial build with PCH disabled (required for compile caching), then modify KeybindManager.hpp and corresponding .cpp to introduce a new dispatcher, and finally incrementally rebuild the modified files.
- main branch:
- 49 files to rebuild (67 total)
- cpu time: avg 120s (single thread)
- PR:
- 12 files to rebuild
- cpu time: avg 33.3s (single thread)
Speed up: avg 350%
Clearly defined dependencies
Continuing with the example above, in the main branch it is unclear how many files depend on KeybindManager, even though there are only three instances of `#include "KeybindManager.hpp", a change in the header results in in 49 modified files.
In this PR, the dependant of KeybindManager can be easly found using a simple grep [^1],
$ grep KeybindManager.hpp -lR src | grep -v pch
src/config/ConfigManager.cpp
src/debug/HyprCtl.cpp
src/events/Windows.cpp
src/helpers/Monitor.cpp
src/layout/DwindleLayout.cpp
src/layout/MasterLayout.cpp
src/layout/IHyprLayout.cpp
src/managers/input/InputManager.cpp
src/managers/KeybindManager.cpp
src/plugins/PluginAPI.cpp
src/Window.cpp
src/Compositor.cpp
[^1]: Some are not hard dependencies and further improvements can be made.
yes. The problem with this is
idk about the "chaotic" part, if I have 30 includes on top of every file, to me, that is even more chaotic.
We could absolutely eliminate almost all the includes in each cpp file by just including a single pch.hpp file that includes everything else, but that would complicate the dependency. It is chaotic because of how the individual units are linked together.
e.g.
ninja -C build -t graph
If lines of code is important as a metric, I think another approach is to include all the use std and dependency library headers into a single common header and include it in each required file instead.
But separating the headers owned by Hyprland could still be valuable for development, as it means fewer rebuilds and fewer complaints from the language server.
well, including all the stl shit into a common.hpp file would be bad the other way - increase compile times massively. Up till now I have been trying to strike a balance between the two.
There is another more conservative branch which only touches headers from /src, the diff should be half this size.
wait, since this is just a util, can't we make it run before build, write the new source files to ./build and compile that? I believe CMake would not have issues with that
You mean something like LLVM's clang-include-cleaner/fixer or Google's iwyu? Both run as slow as C++ compilers, if not slower.
Just personal opinion, I believe the best practice is still to include only and directly what it uses. Maybe step by step, but new include should follow the rule.
Share common includes in several headers maybe good enough at the beginning, but it's likely to cause more trouble as time goes by.
Tens of includes on top of every file may be chaotic to some degree, but it's still necessary code to keep things explicit and straight forward. Just like the code for other functions or classes, it's there for a reason, and can be ignored too when you're focused on something else.
