mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 64743: Enabled function sections.
Date Wed, 20 Dec 2017 10:52:18 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64743/#review194224
-----------------------------------------------------------




cmake/CompilationConfigure.cmake
Line 262 (original), 262 (patched)
<https://reviews.apache.org/r/64743/#comment272943>

    Since we have a configure flag to disable these in autotools land, let's also add some
cmake option to turn this off.



cmake/CompilationConfigure.cmake
Lines 263 (patched)
<https://reviews.apache.org/r/64743/#comment272940>

    Let's not depend on the platform, but instead on the toolchain instead. This does work
on e.g., macos.



cmake/CompilationConfigure.cmake
Lines 264-265 (patched)
<https://reviews.apache.org/r/64743/#comment272944>

    Let's merge these lines.



cmake/CompilationConfigure.cmake
Lines 267 (patched)
<https://reviews.apache.org/r/64743/#comment272942>

    What about static archives? Last time I looked this was broken in the cmake setup, but
it would be great to prepare to also make use of this flag there once its ready (if applicable).



configure.ac
Lines 178 (patched)
<https://reviews.apache.org/r/64743/#comment272939>

    We already have two forms for handling of configure default args, one explicit (e.g.,
right above), and another one for e.g., `parallel-test-execution` or `werror`.
    
    I'd suggest to just follow the flow of `werror` instead of introducing yet another approach.
That would make the eventual cleanup possibly less upsetting.



configure.ac
Lines 507-508 (patched)
<https://reviews.apache.org/r/64743/#comment272938>

    Why are we disabling this on macos? These flags do work with the latest LLVM toolchain
(clang & lld), so let's default-enable it on macos as well.
    
    If you worry about some Apple clang fork not supporting these flags, let's please add
some feature detection instead of branching on the platform here.


- Benjamin Bannier


On Dec. 20, 2017, 4:50 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64743/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2017, 4:50 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8348
>     https://issues.apache.org/jira/browse/MESOS-8348
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If we tell the compiler to place each function in a separate
> section, this allows the linker to garbage collect unused
> sections. This significantly decreases the size of the final
> build artifacts and provides some modest improvements in build
> times.
> 
> This feature is currently only implemented in Linux toolchains,
> so we automatically enable it only on Linux.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake dc9dc161dce1c714748bcec2fc15c4fbfbccaec2 
>   configure.ac 5ffc33a6b9495db64f1ec164ed96f3faea620348 
> 
> 
> Diff: https://reviews.apache.org/r/64743/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message