[llvm-dev] RFC: Large, especially super-linear, compile time regressions are bugs.

Thu Mar 31 13:41:43 PDT 2016

Hi,

TLDR: I totally support considering compile time regression as bug.

I'm glad you bring this topic. Also it is worth pointing at this recent thread: http://lists.llvm.org/pipermail/llvm-dev/2016-March/096488.html
And also this blog post comparing the evolution of clang and gcc on this aspect: http://hubicka.blogspot.nl/2016/03/building-libreoffice-with-gcc-6-and-lto.html

I will repeat myself here, since we also noticed internally that compile time was slowly degrading with time. Bruno and Chris are working on some infrastructure and tooling to help tracking closely compile time regressions.

We had this conversation internally about the tradeoff between compile-time and runtime performance, and I planned to bring-up the topic on the list in the coming months, but was waiting for more tooling to be ready.
Apparently in the past (years/decade ago?) the project was very conservative on adding any optimizations that would impact compile time, however there is no explicit policy (that I know of) to address this tradeoff. 
The closest I could find would be what Chandler wrote in: http://reviews.llvm.org/D12826 <http://reviews.llvm.org/D12826> ; for instance for O2 he stated that "if an optimization increases compile time by 5% or increases code size by 5% for a particular benchmark, that benchmark should also be one which sees a 5% runtime improvement".

My hope is that with better tooling for tracking compile time in the future, we'll reach a state where we'll be able to consider "breaking" the compile-time regression test as important as breaking any test: i.e. the offending commit should be reverted unless it has been shown to significantly (hand wavy...) improve the runtime performance.

Since you raise the discussion now, I take the opportunity to push on the "more aggressive" side: I think the policy should be a balance between the improvement the commit brings compared to the compile time slow down. Something along the line as what you wrote in my quote above.
You are referring to "large" compile time regressions (aside: what is "large"?), while Bruno has graphs that shows that the compile time regressions are mostly a lot of 1-3% regressions in general, spread over tens of commits. 
Also (and this where we need better tooling) *unexpected* compile-time slow down are what makes me worried: i.e. the author of the commit adds something but didn't expect the compile time to be "significantly" impacted. This is motivated by Bruno/Chris data.
Tracking this more closely may also help to triage thing between O2 and O3 when a commit introduces a compile time slow-down but also brings significant enough runtime improvements.

-- 
Mehdi

> On Mar 31, 2016, at 1:28 PM, Chandler Carruth via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> LLVM has a wonderful policy regarding broken commits: we revert to green. We ask that a test case be available within a reasonable time frame (preferably before, but some exceptions can be made), but otherwise we revert the offending patch, even if it contains nice features that people want, and keep the tree green. This is an awesome policy.
> 
> I would like to suggest we adopt and follow the same policy for compile time regressions that are large, and especially for ones that are super-linear. As an example from the previous thread:
> 
> On Mon, Mar 14, 2016 at 12:14 PM Bruno Cardoso Lopes via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> > There is a possibility that r259673 could play a role here.
> >
> > For the buildSchedGraph() method, there is the -dag-maps-huge-region that
> > has the default value of 1000. When I commited the patch, I was expecting
> > people to lower this value as needed and also suggested this, but this has
> > not happened. 1000 is very high, basically "unlimited".
> >
> > It would be interesting to see what results you get with e.g. -mllvm
> > -dag-maps-huge-region=50. Of course, since this is a trade-off between
> > compile time and scheduler freedom, some care should be taken before
> > lowering this in trunk.
> 
> Indeed we hit this internally, filed a PR:
> https://llvm.org/bugs/show_bug.cgi?id=26940 <https://llvm.org/bugs/show_bug.cgi?id=26940>
> 
> I think we should have rolled back r259673 as soon as the test case was available.
> 
> Thoughts?
> _______________________________________________
> 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/20160331/94af3e0d/attachment.html>