Don't include TMathBase.h in TString.h #19591
Conversation
guitargeek
requested review from
dpiparo,
martamaja10,
vepadulano,
pcanal,
lmoneta,
bellenot and
couet
as code owners
guitargeek
force-pushed
the
tstring_tmathbase
branch
from
633fe10 to
1e7c50e
Compare
guitargeek
force-pushed
the
tstring_tmathbase
branch
2 times, most recently
from
92e4877 to
17330f8
Compare
guitargeek
force-pushed
the
tstring_tmathbase
branch
2 times, most recently
from
8756196 to
af18310
Compare
Test Results 21 files 21 suites 3d 10h 46m 11s ⏱️ For more details on these failures, see this check. Results for commit 27c4e9a. ♻️ This comment has been updated with latest results. |
guitargeek
force-pushed
the
tstring_tmathbase
branch
5 times, most recently
from
bbf141a to
c3e8799
Compare
|
Not specific to this PR, but more in general: can an automated way be envisaged to spot similar opportunities for improving build time? Also because in ROOT buildtime and runtime, thanks to C++ interpretation, have a very much blurred borders... |
guitargeek
force-pushed
the
tstring_tmathbase
branch
from
c3e8799 to
fc7ec84
Compare
That's a good question. I don't know how to automatize this, but I have manually identified a few spots that I might address later. By the way, the motivation for this PR were RooFit compile times in particular. RooFit is a pretty huge codebase inside ROOT, and compile times were creeping up gradually. Also because it has so many test binaries... I'll try to get this under control gradually, but changes should also not be too aggressive. Users might also rely on these indirect inclusions and get annoyed when the code doesn't compile anymore. The removal of |
guitargeek
force-pushed
the
tstring_tmathbase
branch
3 times, most recently
from
3a25c19 to
aa6820f
Compare
|
Given the magnitude of the change, does it make sense to already add some wording to the RNs? |
The TString header only uses `TMath` functions that are readily available in the standard library. The `TMathBase.h` header is quite big, and avoiding to include it reduces the overall cold-ccache compile time of the full ROOT project with all default features by about 2 % on my machine, namely from 870 s to 855 s on average. In the places where the `TMath` functions were used but only indirectly included via `TString.h` inclusion, the commit suggests to use the `<cmath>` functions instead.
guitargeek
force-pushed
the
tstring_tmathbase
branch
from
aa6820f to
27c4e9a
Compare
This fixes compilation failure after the following ROOT PR: * root-project/root#19591
The TString header only uses
TMathfunctions that are readily available in the standard library.The
TMathBase.hheader is quite big, and avoiding to include it reduces the overall cold-ccache compile time of the full ROOT project (excluding LLVM/Clang) with all default features by about 2 % on my machine, namely from 870 s to 855 s on average (I repeated the measurement several times, they are reproducible to a second).In the places there the
TMathfunctions were used but only indirectly included viaTString.hinclusion, the commit suggests to use the<cmath>functions instead.