[llvm-dev] [cfe-dev] Who wants faster LLVM/Clang builds?

Fri Dec 15 10:58:48 PST 2017

2017-12-09 12:54 GMT-08:00 Chris Lattner via llvm-dev <
llvm-dev at lists.llvm.org>:

>
>
> On Dec 8, 2017, at 5:01 PM, Mikhail Zolotukhin via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi,
>
> I tweaked my scripts to avoid removing includes when it doesn't give any
> significant benefits, which made the patches significantly smaller. This
> time the patches should not try to remove includes of header files, which
> are transitively included from other included header files. The gains
> mostly remained the same (plus/minus noise), the tables are in the end of
> the email. I also included size of preprocessed files (measured in 1000
> lines of code).
>
>
> Just my opinion, but it is *good* to remove redundant header file
> includes, even if it doesn’t speed up the build.
> http://llvm.org/docs/CodingStandards.html#minimal-list-of-includes
>

Do you consider transitive includes as redundant? Why should we remove
these?
One drawback I see with removing these is that if I need `class Foo` but I
get it through `bar.h` which includes `foo.h`, it means that removing the
include to `foo.h` in `bar.h` would break the client of bar that relied on
this transitive includes.
That makes refactoring of a components harder since it may break some of
the clients of the components for no other reason than this transitive
include.
I suspect this is why IWYU would instead *add* these includes when they're
missing.

-- 
Mehdi

>
>
> I suggest that from here we go as follows: maintainers/interested people
> take a look at files related to their components and pick the parts of the
> patches that they consider correct. I'll also start with some files next
> week if there is no objections to it. Does it sound reasonable?
>
> The most impacted files (the numbers are for Debug build):
>
> *LLVM top 10*
> *Filename TimeOld TimeNew Delta **SizeOld SizeNew SizeDelta*
> lib/CodeGen/GlobalISel/GlobalISel.cpp 0.26 0.02 -91.6% 35.0 0.3 -99.0%
> lib/MC/MCLabel.cpp 0.20 0.02 -88.0% 25.5 0.0 -99.9%
> tools/llvm-readobj/ObjDumper.cpp 0.44 0.10 -76.8% 41.0 11.8 -71.1%
> lib/MC/MCWinEH.cpp 0.49 0.15 -70.4% 43.9 21.4 -51.2%
> lib/Transforms/Vectorize/Vectorize.cpp 0.73 0.29 -60.7% 52.7 35.5 -32.6%
> tools/llvm-diff/DiffLog.cpp 0.59 0.27 -53.8% 50.7 33.7 -33.7%
> lib/Target/ARM/MCTargetDesc/ARMMachORelocationInfo.cpp 0.47 0.25 -46.7%
> 46.7 37.9 -18.9%
> lib/DebugInfo/DWARF/DWARFExpression.cpp 0.67 0.38 -43.5% 47.4 34.8 -26.7%
> lib/Transforms/Utils/ASanStackFrameLayout.cpp 0.52 0.32 -38.8% 41.7 33.7
> -19.2%
> tools/llvm-dwp/llvm-dwp.cpp 2.48 1.53 -38.3% 92.5 55.2 -40.3%
>
> Full list:
> <llvm.txt>
>
> *Clang top 10*
> *Filename TimeOld TimeNew Delta SizeOld SizeNew SizeDelta*
> tools/libclang/CXString.cpp 2.25 0.30 -86.9% 85.1 31.7 -62.7%
> lib/Tooling/CommonOptionsParser.cpp 2.33 0.68 -70.6% 87.1 34.4 -60.5%
> lib/AST/StmtViz.cpp 1.28 0.48 -62.5% 62.4 39.0 -37.5%
> tools/driver/cc1_main.cpp 3.05 1.29 -57.8% 93.7 58.6 -37.4%
> unittests/CodeGen/BufferSourceTest.cpp 4.12 2.62 -36.5% 145.8 103.9 -28.7%
> lib/CodeGen/CGLoopInfo.cpp 2.43 1.68 -30.7% 101.6 82.5 -18.8%
> unittests/CodeGen/CodeGenExternalTest.cpp 4.50 3.21 -28.6% 155.5 125.1
> -19.5%
> lib/Driver/ToolChains/Contiki.cpp 0.53 0.38 -28.1% 42.4 38.0 -10.5%
> unittests/Tooling/RefactoringActionRulesTest.cpp 3.22 2.34 -27.5% 108.3
> 90.0 -16.9%
> lib/Serialization/GeneratePCH.cpp 2.38 1.78 -25.1% 83.8 71.1 -15.1%
>
> Full list:
> <clang.txt>
>
>
> The updated patches:
> <llvm_redundant_headers.patch>
> <clang_redundant_headers.patch>
>
> Thanks,
> Michael
>
> On Dec 8, 2017, at 9:20 AM, Quentin Colombet via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>
>
> On Dec 6, 2017, at 1:17 PM, Matthias Braun via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> - We do indeed have a lot of unnecessary includes around in llvm (or
> pretty much any other C++ project for that matter).
> - I want faster builds.
> - The only way to reliably fight this is indeed automatic tools.
> - Having the right amount of includes also has documentation value and
> ideally let's you understand the structure of your project.
> - However relying on transitive includes works contrary to the last
> "undestanding/documentation" point.
> - (And as stated earlier to have things really clean we want `class XXX;`
> instead of `#include "XXX.h"` wherever possible. And if you are serious
> about that we also often have to reduce the amount of include code in the
> headers so we can move the `#include "XXX.h"` from the header to the
> implementation.
>
> For me personally I think the documentation/understandability we loose
> when relying on transitive includes weights heavier than my desire to get a
> faster build…
>
>
> +1
>
> Q.
>
>
> - Matthias
>
> On Dec 6, 2017, at 12:28 PM, serge guelton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> On Wed, Dec 06, 2017 at 11:38:54AM -0800, Michael Zolotukhin via llvm-dev
> wrote:
>
>
> On Dec 6, 2017, at 9:00 AM, mats petersson via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> In my experience, a lot of time is spent on optimizing the code (assuming
> it's not a "-O0" build).
>
> The numbers were actually for the debug build (-O0 -g), so for Release
> build they would be different (presumably lower).
>
> Also redundant includes are largely fixed by header guards, and I believe
> Clang [and gcc as well as MS Compilers, and probably most others too] have
> an include guards-cache that determines that "we've already included foo.h,
> and it has include guards around the whole actual content of the file, so
> we can just skip it”.
>
> By redundant here I meant that we included a file, but we didn’t use any
> of its content (rather than we included the same file twice).
>
>
> So I'm slightly dubious as to this being an efficient way of significantly
> reducing the total compilation time for the overall project - even if there
> are SOME cases where there is a significant improvement in a single file.
> The total time for a clean build [in wall-clock-time, not CPU-time] should
> be measured, making sure that there is enough memory. Doing a run of, say,
> five complete builds of the same thing [with suitable "clean" between to
> redo the whole build], take away the worst and the best, and perhaps also
> "modify one of the more common header files" (llvm/IR/Type.h for example)
> and build again.
>
> On full builds the benefit is not big (around 1%, but the noise is high),
> but: 1) if we only take gains more than, say, 5%, we’ll probably never see
> any, 2) I aim at changes that make the code strictly better (modulo David’s
> point about disk cache). If any change is questionable from maintenance or
> whatever other point of view, I’m all for dropping it.
>
>
> my 2¢
>
> +1 for point 2). Even leaving aside the speed gain, removing unused
> includes file just looks like good coding practice to me.
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171215/d099ff40/attachment.html>