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

Proposed by Alexandros Frantzis on 2016-08-04
Status: Merged
Approved by: Daniel van Vugt on 2016-08-08
Approved revision: 3630
Merged at revision: 3634
Proposed branch: lp:~afrantzis/mir/no-lto-for-tests
Merge into: lp:mir
Diff against target: 28 lines (+5/-2)
2 files modified
CMakeLists.txt (+2/-2)
tests/CMakeLists.txt (+3/-0)
To merge this branch: bzr merge lp:~afrantzis/mir/no-lto-for-tests
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve on 2016-08-08
Cemil Azizoglu (community) Approve on 2016-08-05
Kevin DuBois (community) 2016-08-04 Approve on 2016-08-04
Mir CI Bot continuous-integration Approve on 2016-08-04
Review via email: mp+302034@code.launchpad.net

Commit message

tests: Don't build with LTO

Description of the change

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.

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3630
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:

review: Approve (continuous-integration)
Kevin DuBois (kdub) wrote :

alright by me

review: Approve
Daniel van Vugt (vanvugt) wrote :

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) wrote :

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) wrote :

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) wrote :

> 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) wrote :

I'm ok with it.

review: Approve
Daniel van Vugt (vanvugt) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-08-02 03:44:01 +0000
3+++ CMakeLists.txt 2016-08-04 14:42:27 +0000
4@@ -84,8 +84,8 @@
6 option(MIR_LINK_TIME_OPTIMIZATION "Enables the linker to optimize binaries." OFF)
8- set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -flto")
10+ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -flto -ffat-lto-objects")
11+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto -ffat-lto-objects")
13 set(CMAKE_NM "gcc-nm")
14 set(CMAKE_AR "gcc-ar")
16=== modified file 'tests/CMakeLists.txt'
17--- tests/CMakeLists.txt 2016-08-01 14:43:19 +0000
18+++ tests/CMakeLists.txt 2016-08-04 14:42:27 +0000
19@@ -25,6 +25,9 @@
20 set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-inconsistent-missing-override")
21 endif()
23+set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-lto")
24+set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-lto")
27 add_definitions(-DANDROID)
28 endif()


People subscribed via source and target branches