We have many other headers including <algorithm>; it would be interesting
to compare the percentage of our cpp files that recursively include
<algorithm> before and after that patch; I suppose that just a single patch
like that is not enough to move that needle much, because there are other
ways that <algorithm> gets included in the same cpp files.
I do expect, though, that the 23 ms overhead from including <algorithm> is
real (at least as an order of magnitude), so I still expect that we can
save 23 ms times the number of cpp files that currently include <algorithm>
and could avoid to.
Those 9,000 lines of code are indeed a moderate amount of extra code
compared to the million lines of code that we have in many compilation
units, and yes I think we all expect that there are bigger wins to make.
This one is a relatively easy one though and 9,000 lines, while moderate,
is not negligible. How many other times are we neglecting 9,000 lines and
to how much does that add up? In the end, I believe that the ratio
(number of useful lines of code) / (total lines of code included)
is a very meaningful metric, and including <algorithm> for min/max scores
less than 1e-3 on that metric.
Benoit
2013/9/8 Nicholas Cameron <nick.r.came...@gmail.com>
> I timed builds to see if this makes a significant difference and it did
> not.
>
> I timed a clobber debug build using clang with no ccache on Linux on a
> fast laptop. I timed using a pull from m-c about a week old (I am using
> this pull because I have a lot of other stats on it). I then applied
> bjacob's nscoord patch from the bug and a patch of my own which does a
> similar thing for some Moz2D headers which get pulled into a lot of files
> (~900 other headers, presumably more cpp files). For both runs I did a full
> build, then clobbered, then timed a build. I avoided doing any other work
> on the laptop. n=1, so there might be variation, but my experience with
> build times is that there usually isn't much.
>
> Before changes:
>
> real 38m54.373s
> user 234m48.508ms
> sys 7m18.708s
>
> after changes:
>
> real 39m11.123s
> user 234m26.864ms
> sys 7m10.336s
>
> The removed headers are also the ideal case for ccache, so incremental
> builds or real life clobber builds should be affected even less by these
> changes.
>
> I don't think these kind of time improvements make it worth duplicating
> std library code into mfbt, we may as well just pull in the headers and
> forget about it. A caveat would be if it makes a significant difference on
> slower systems.
>
> Given that improving what gets included via headers can make significant
> difference to build time, this makes me wonder exactly what aspect of
> header inclusion (if not size, which we should catch here) makes the
> difference.
>
> Nick.
>
> On Sunday, September 8, 2013 3:22:01 PM UTC+12, Benoit Jacob wrote:
> > Hi,
> >
> >
> >
> > It seems that we have some much-included header files including
> <algorithm>
> >
> > just to get std::min and std::max.
> >
> >
> >
> > That seems like an extreme case of low ratio between lines of code
> included
> >
> > (9,290 on my system, see Appendix below) and lines of code actually used
> >
> > (say 6 with whitespace).
> >
> >
> >
> > I ran into this issue while trying to minimize nsCoord.h (
> >
> > https://bugzilla.mozilla.org/show_bug.cgi?id=913868 ) and in my patch, I
> >
> > resorted to defining my own min/max functions in a nsCoords_details
> >
> > namespace.
> >
> >
> >
> > This prompted comments on that bug suggesting that it might be better to
> >
> > have that in MFBT. But that, in turn, sounds like overturning our recent
> >
> > decision to switch to std::min / std::max, which I feel is material for
> >
> > this mailing list.
> >
> >
> >
> > It is also conceivable to keep saying that we should use std::min /
> >
> > std::max *except* in headers that don't otherwise include <algorithm>,
> >
> > where it may be more reasonable to use the cheap-to-#include variant
> >
> > instead.
> >
> >
> >
> > What do you think?
> >
> >
> >
> > Benoit
> >
> >
> >
> > === Appendix: how big and long to compile is <algorithm> ? ===
> >
> >
> >
> > On my Ubuntu 12.04 64bit system, with GCC 4.6.3, including <algorithm>
> >
> > means recursively including 9,290 lines of code:
> >
> >
> >
> > $ echo '#include<algorithm>' > a.cpp && g++ -save-temps -c a.cpp && wc -l
> >
> > a.ii
> >
> > 9290 a.ii
> >
> >
> >
> > On may wonder what this implies in terms of compilation times; here is a
> >
> > naive answer. I'm timing 10 successive compilations of a file that just
> >
> > includes <iostream>, and then I do the same with a file that also
> includes
> >
> > <algorithm>.
> >
> >
> >
> > $ echo '#include<iostream>' > a.cpp && time (g++ -c a.cpp && g++ -c a.cpp
> >
> > && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c
> >
> > a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp)
> >
> >
> >
> > real 0m1.391s
> >
> > user 0m1.108s
> >
> > sys 0m0.212s
> >
> >
> >
> > echo '#include<algorithm>' > a.cpp && echo '#include<iostream>' >> a.cpp
> &&
> >
> > time (g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++
> >
> > -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp
> &&
> >
> > g++ -c a.cpp)
> >
> >
> >
> > real 0m1.617s
> >
> > user 0m1.324s
> >
> > sys 0m0.244s
> >
> >
> >
> > (I actually repeated this many times and kept the best result for each;
> my
> >
> > hardware is a Thinkpad W520 with a 2.5GHz, 8M cache Core i7).
> >
> >
> >
> > So we see that adding the #include<algorithm> made each compilation 23 ms
> >
> > longer in average (226 ms for 10 compilations).
>
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
