On 23.09.24 16:57, ichthyo wrote:
Another issue that was pointed out on the Github pull request
https://github.com/Yoshimi/yoshimi/pull/207#pullrequestreview-2322393639
As part of my rework, I used some generic helpers and created a new header
Util.hpp. There is a function using std::set in this header, and basically I
wanted to avoid including <set> directly, since then this include would be
pulled into a large part of the whole code base.
We know from experience that header includes can have a huge impact on build
times. Just recently Kristian did some re-orderings at the global level, with
the result that Yoshimi now builds at least three times faster, which makes
a huge difference in everyday work.
I had similar experirences on several occasions in the past.
But I must admit, that I acted based on *assumption* in this case,
which is not what an engineer should do.
The reality is: the structure of the includes in yoshimi is far from optimised
in any way. We have some very large code entities (Config, and SynthEngine),
which on the one hand are feature rich, while on the other hand are also
referred to from almost everywhere. So these are kind of "super spreaders",
since they pull in lots of dependencies already, even while the header uses
quite some forward declarations. Add to this that the downstream headers
are also not included with a star-shaped topology, but have quite some
cross usages.
So this gives rise to the suspicion, that we are engaged into a phantom
discussion here and a treatment of imaginary pains.
And a quick empirical test confirms this suspicion.
If we include <set> and <algorithm> right into globals.h,
the effect on build time is *negligible*
Both user time and real end-to-end time show a statistic spread of roughly
0.8sec in my measurements. And within those bounds, there is no difference
between those scenarios.
Observed values on Release build are:
User time ~ 7min 20.3±0.4 sec and
Real time ~ 1min 5.3±0.4 sec (with 8 cores and -j16 )
So the bottom line is: we can forget about that *premature optimisation*
regarding compile times and simple replace the forward declaration by an
#include <set>
In the current code base, an #include <algorithm> should not be necessary,
since the ambiguity is related to std::set and the std::find algorithm
is not used on a signature but only in the implementation (which the compiler
does not type check unless it is actually instantiated)
-- Hermann
PS: That being said -- we should still consider / discuss the structure of
the "Util.h". I introduced this driven by immediate convenience, and did
not think too much. I'm under the impression that the utils defined there
could be relocated into existing utility headers.
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
