Merge lp:~afrantzis/mir/no-lto-for-tests into lp:mir

[Alexandros Frantzis]

tests: Don't build with LTO

tests: Don't build with LTO

We don't care about our tests being link-time-optimized, and there are two main benefits in disabling it:

1. Improved build time (~20% improvement in parallel builds vs full-lto)
2. No need to care about various obscure issues either caused or uncovered by LTO in our test code (gcc-6 seems to have a bunch of these).

Sample runs on my i5 quad-core using debflags and ld.gold:

                          no-test-lto full-lto no-lto
build-time (real/user) : 13m/45m 16m/43m 12m/43m
build-dir-size : 3.1 GiB 1.8 GiB 3.0 GiB

Note that the 'user' build-time is slightly higher for the no-test-lto case because we are producing "fat" objects for our non-test code (objects that have both binary code and intermediate bytecode and can be linked both with and without LTO). However, the work is distributed across all compilations and can be parallelized more efficiently, hence the lower 'real' time. In contrast, in the full-lto case most of the work is performed at the linking stage which cannot be parallelized.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3630
https://mir-jenkins.ubuntu.com/job/mir-ci/1397/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1700
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1753
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1744
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1744
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1744
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1720/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1720/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1720/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1720/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1720/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1720
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1720/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1397/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

alright by me

review: Approve

[Daniel van Vugt (vanvugt)]

Sounds reasonable.

We should revisit bug 1473330 at some point. Well-written C code will typically have a bloat factor of binaries 1-3x the size of their source code. So it stands to reason that C++ shouldn't be much more than an order of magnitude worse than that. The Mir code is presently several orders of magnitude worse right now, so we should look at ways to further reduce bloat in future.

review: Approve

[Daniel van Vugt (vanvugt)]

Hmm, can we do this for "debug" builds only, if there is such a thing?

It would be nice to get a reduced footprint in releases still.

review: Needs Information

[Daniel van Vugt (vanvugt)]

Alternatively, if there is no distinction between debug and release (which is a perfectly valid position) then I think there's an argument that we don't want this change. Because it only speeds up development slightly, but at the cost of more bloated releases.

Are there any open bugs that would be fixed by this branch?

review: Needs Information

[Alexandros Frantzis (afrantzis)]

> Are there any open bugs that would be fixed by this branch?

There is https://bugs.launchpad.net/mir/+bug/1610215 . In parallel I am trying to fix the problems mentioned in the bug while keeping LTO enabled for tests, however it's proving to be tricky.

[Cemil Azizoglu (cemil-azizoglu)]

I'm ok with it.

review: Approve

[Daniel van Vugt (vanvugt)]

OK then. I'm fine with disabling an optimisation to fix bug 1610215.

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2016-08-02 03:44:01 +0000
+++ CMakeLists.txt	2016-08-04 14:42:27 +0000
@@ -84,8 +84,8 @@
 
 option(MIR_LINK_TIME_OPTIMIZATION "Enables the linker to optimize binaries." OFF)
 if(MIR_LINK_TIME_OPTIMIZATION)
-  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -flto")
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -flto -ffat-lto-objects")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto -ffat-lto-objects")
   if(${CMAKE_COMPILER_IS_GNUCXX})
     set(CMAKE_NM "gcc-nm")
     set(CMAKE_AR "gcc-ar")

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-08-01 14:43:19 +0000
+++ tests/CMakeLists.txt	2016-08-04 14:42:27 +0000
@@ -25,6 +25,9 @@
   set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-inconsistent-missing-override")
 endif()
 
+set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-lto")
+set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-lto")
+
 if (MIR_TEST_PLATFORM STREQUAL "android")
   add_definitions(-DANDROID)
 endif()