<div dir="auto">Hey all,<div dir="auto"><br></div><div dir="auto">IWYU maintainer here. I wanted to make a small observation.</div><div dir="auto"><br></div><div dir="auto">Surprisingly, IWYU will most often *add* includes to a reasonably well-factored codebase, and this ties into Chris' comment:</div><div dir="auto"><br></div><div dir="auto"><span style="font-family:sans-serif;font-size:13.696px">> Beyond that though, this seems like obvious </span></div><div dir="auto"><span style="font-family:sans-serif;font-size:13.696px">> goodness to reduce coupling in the codebase.</span><br></div><div dir="auto"><br></div><div dir="auto">Just blindly removing includes will probably increase coupling, not reduce it, because it optimizes for transitive dependencies.</div><div dir="auto"><br></div><div dir="auto">So if a refactor makes it possible to remove an include in a faraway upstream header, that will potentially break lots of code.</div><div dir="auto"><br></div><div dir="auto">One of IWYU's benefits, imho, is that it works against this. But it's a much harder problem, which is why it's less immediately effective :)</div><div dir="auto"><br></div><div dir="auto">Good to see some numbers below! </div><div dir="auto"><br></div><div dir="auto">- Kim </div></div><div class="gmail_extra"><br><div class="gmail_quote">Den 6 dec. 2017 7:09 fm skrev "Michael Zolotukhin via cfe-dev" <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>>:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Dec 5, 2017, at 9:59 PM, Justin Lebar <<a href="mailto:jlebar@google.com" target="_blank">jlebar@google.com</a>> wrote:</div><br class="m_-9090650441335892303Apple-interchange-newline"><div><div dir="ltr">> To find which header files could be removed it scanned the file for "#include" lines and tried to remove them one by one (checking if the file still compiles after the removal). When there were no more include lines to remove, we verified the change with ninja+ninja check.<div><br></div><div>It looks like this makes us rely heavily on transitive header includes -- is that right?</div></div></div></blockquote>Correct.<br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>Specifically what I mean is, suppose <a href="http://foo.cc" target="_blank">foo.cc</a> uses class1 and class2. class2.h #includes "class1.h". If <a href="http://foo.cc" target="_blank">foo.cc</a> had a #include for class1.h and class2.h, this tool will remove the include of class1.h. Is that right?</div></div></div></blockquote>Correct.<br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>If so, seems like probably more aggressive than we want to be, because it makes our codebase fragile: If someone removes class1.h from class2.h, it may break <a href="http://foo.cc" target="_blank">foo.cc</a>.</div></div></div></blockquote>I completely agree, that’s why I intended this patch to be a collection of hints for real people rather than something we want to commit as-is. I think I can make the tool check the patch again and try to restore #include lines that don’t contribute to the speedup.</div><div><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>I'm sure we can find many situations like this today, but this approach seems it would make that situation much more common.</div><div><br></div><div>IWYU would avoid this pitfall, although I know IWYU has its own problems. Alternatively maybe if the tool only removed headers whose removal significantly decreased the preprocessed size of the file, we'd know that we actually removed "something interesting”.</div></div></div></blockquote>That’s a good idea. Thanks for the feedback!</div><div><br></div><div>Michael<br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>-Justin</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 5, 2017 at 9:38 PM Chris Lattner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">I, for one, want faster builds.<div><br></div><div>Beyond that though, this seems like obvious goodness to reduce coupling in the codebase. I’ve only skimmed the patch, but this seems like a clearly amazingly great ideas. Did you use the IWYU tool or something else?</div><div><br></div><div>-Chris</div><div><br><div><br><blockquote type="cite"></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div>On Dec 5, 2017, at 3:40 PM, Mikhail Zolotukhin via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="m_-9090650441335892303m_-7432678922244347213Apple-interchange-newline"></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><div style="word-wrap:break-word">Hi,<div><br></div><div>Recently I've done some experiments on the LLVM/Clang code and discovered that many of our source files often include unnecessary header files. I wrote a simple tool that eliminates redundant includes and estimates benefits of doing it, and the results were quite nice: for some files we were able to save 90% of compile time! I think we want to apply some of the cleanups I found, but I'm not sure how to better do it: the total patches are 8k lines of code for LLVM and 3k lines of code for clang (I'll attach them for reference). My suggestion would be that people take a look at the list of changed files and pick the changes for the piece of code they are working on if the changes look sane (the changes do need some checking before committing). Does it sound like a good idea? I'd appreciate any feedback on what can we do here.</div><div><br></div><div>The list of files for which removing redundant headers improved compile time (the numbers are compile time in seconds for a Debug build):</div><div><b><br></b></div><div><b>LLVM top 10</b></div><div><div><font face="Menlo" style="font-size:11px"><b><u>Filename<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>Old<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>New<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>Delta</u></b></font></div><div><font face="Menlo" style="font-size:11px">lib/CodeGen/GlobalISel/<wbr>GlobalISel.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.26<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.02<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-91.9%</font></div><div><font face="Menlo" style="font-size:11px">lib/MC/MCLabel.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.19<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.02<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-88.2%</font></div><div><font face="Menlo" style="font-size:11px">tools/llvm-readobj/ObjDumper.<wbr>cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.43<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.10<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-76.5%</font></div><div><font face="Menlo" style="font-size:11px">lib/MC/MCWinEH.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.51<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.13<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-74.3%</font></div><div><font face="Menlo" style="font-size:11px">lib/Transforms/Vectorize/<wbr>Vectorize.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.72<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.29<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-59.7%</font></div><div><font face="Menlo" style="font-size:11px">tools/llvm-diff/DiffLog.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.58<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.26<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-54.6%</font></div><div><font face="Menlo" style="font-size:11px">lib/Target/ARM/MCTargetDesc/<wbr>ARMMachORelocationInfo.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.46<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.26<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-44.1%</font></div><div><font face="Menlo" style="font-size:11px">lib/DebugInfo/DWARF/<wbr>DWARFExpression.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.68<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.38<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-43.3%</font></div><div><font face="Menlo" style="font-size:11px">lib/LTO/LTOModule.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>2.25<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.33<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-41.1%</font></div><div><font face="Menlo" style="font-size:11px">lib/Target/TargetMachine.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.76<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.10<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-37.8%</font></div></div><div><br></div><div>Full list:</div><div></div></div></div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><span id="m_-9090650441335892303m_-7432678922244347213cid:9A8C3012-C316-4FA8-A003-6146C517AC98@wp.comcast.net"><llvm.txt></span></div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><div style="word-wrap:break-word"><div></div><div><br></div><div><br></div><div><b>Clang top 10</b></div><div><div><font face="Menlo" style="font-size:11px"><b><u>Filename<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>Old<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>New<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>Delta</u></b></font></div><div><span style="font-size:11px;font-family:Menlo">tools/libclang/CXString.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.70<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.25<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-85.2%</span></div></div><div><font face="Menlo" style="font-size:11px"><div>lib/Tooling/<wbr>CommonOptionsParser.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.69<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.55<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-67.3%</div><div>lib/AST/StmtViz.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.02<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.44<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-57.4%</div><div>tools/driver/cc1_main.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>2.26<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>0.97<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-57.1%</div><div>unittests/CodeGen/<wbr>BufferSourceTest.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>3.08<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.83<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-40.6%</div><div>lib/CodeGen/CGLoopInfo.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.91<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.34<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-29.9%</div><div>unittests/Tooling/<wbr>RefactoringActionRulesTest.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>2.46<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.79<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-27.0%</div><div>unittests/CodeGen/<wbr>CodeGenExternalTest.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>3.43<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>2.52<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-26.5%</div><div>tools/libclang/<wbr>CXStoredDiagnostic.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.67<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.26<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-24.8%</div><div>tools/clang-func-mapping/<wbr>ClangFnMapGen.cpp<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>2.48<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>1.89<span class="m_-9090650441335892303m_-7432678922244347213Apple-tab-span" style="white-space:pre-wrap"> </span>-23.8%</div><div><div style="font-family:Helvetica;font-size:12px"><br></div><div style="font-family:Helvetica;font-size:12px">Full list:</div></div><div style="font-family:Helvetica;font-size:12px"></div></font></div></div></div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><span id="m_-9090650441335892303m_-7432678922244347213cid:149AB015-99FC-4554-8B3C-255969944171@wp.comcast.net"><clang.txt></span></div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><div style="word-wrap:break-word"><div><font face="Menlo" style="font-size:11px"><div style="font-family:Helvetica;font-size:12px"></div><div style="font-family:Helvetica;font-size:12px"><br></div><div><span style="font-family:Helvetica;font-size:12px">The corresponding patches (careful, they are big):</span></div><div></div></font></div></div>
</div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><span id="m_-9090650441335892303m_-7432678922244347213cid:09AA8F05-10D5-4013-9A31-BB2AAA58F983@wp.comcast.net"><llvm_redundant_headers.patch></span><div style="word-wrap:break-word"><div><font face="Menlo" style="font-size:11px"><div></div></font></div></div>
<span id="m_-9090650441335892303m_-7432678922244347213cid:CF813FF7-78EC-467A-868A-1F9CCB7C296F@wp.comcast.net"><clang_redundant_headers.<wbr>patch></span></div></blockquote></div></div></div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><blockquote type="cite"><div><div style="word-wrap:break-word"><div><font face="Menlo" style="font-size:11px"><div></div><div><br></div><div><span style="font-family:Helvetica;font-size:12px"><b>Methodology</b></span></div><div><span style="font-family:Helvetica;font-size:12px">My tool took the compile_commands.json from LLVM build and iterated over files trying to remove redundant headers. To find which header files could be removed it scanned the file for "#include" lines and tried to remove them one by one (checking if the file still compiles after the removal). When there were no more include lines to remove, we verified the change with ninja+ninja check. After it we compared preprocessed file size before and after the change hoping to see that it dropped and then checked the compile time impact.</span></div><div><span style="font-family:Helvetica;font-size:12px">NB: As a side effect of this approach we removed all include-lines from inactive "ifdef" sections, which means that the patches <b>*will*</b> break other configurations if applied as-is.</span></div><div><span style="font-family:Helvetica;font-size:12px"><br></span></div><div><span style="font-family:Helvetica;font-size:12px">Thanks,</span></div><div><span style="font-family:Helvetica;font-size:12px">Michael</span></div></font></div></div>______________________________<wbr>_________________<br>LLVM Developers mailing list<br><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br></div></blockquote></div></div></div>______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
</div></blockquote></div><br></div><br>______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div></div>