----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52695/#review154527 -----------------------------------------------------------
I would really like to see actual timings of e.g., an optimized build before and after introducing these new flags, e.g., the runtime of `libprocess-tests` and `benchmarks`. 3rdparty/libprocess/Makefile.am (line 16) <https://reviews.apache.org/r/52695/#comment224111> Remove note about `-Werror` which is not used. 3rdparty/libprocess/Makefile.am (lines 16 - 24) <https://reviews.apache.org/r/52695/#comment224124> I think this would be easier to follow if you'd incrementially build up `AM_CXXFLAGS` while explaining their effect, e.g., # Enable common (and some language specific) warnings. AM_CXXFLAGS += -Wall # Warn when a comparison is made between signed and unsigned values. AM_CXXFLAGS += -Wsign-compare ... 3rdparty/libprocess/Makefile.am (line 29) <https://reviews.apache.org/r/52695/#comment224110> Let's not suppress this valid and potentially useful diagnostic for the whole codebase. It does not trigger a hard failure anyway. 3rdparty/libprocess/Makefile.am (line 30) <https://reviews.apache.org/r/52695/#comment224112> I am not a big fan of unconditionally omitting frame pointers as this gives the optimizer one less register to work with. Unfortunately one cannot easily tell the actual impact of this from the info here. Is this strictly needed here or just nice to have? 3rdparty/libprocess/configure.ac (line 272) <https://reviews.apache.org/r/52695/#comment224114> `s/!$/./` 3rdparty/libprocess/m4/ax_check_compile_flag.m4 (line 1) <https://reviews.apache.org/r/52695/#comment224106> For future updates it would be great if we'd write down the autoconf-archive release this file came from (it looks like the latest release containing it is `v2016.09.16`). - Benjamin Bannier On Nov. 1, 2016, 11:22 p.m., Aaron Wood wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52695/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2016, 11:22 p.m.) > > > Review request for mesos, James Peach, Michael Park, and Neil Conway. > > > Bugs: MESOS-6229 > https://issues.apache.org/jira/browse/MESOS-6229 > > > Repository: mesos > > > Description > ------- > > Use a default set of flags to provide additional security and hardening to > libprocess. Additionally, check and catch more warnings/errors. > > > Diffs > ----- > > 3rdparty/libprocess/Makefile.am 7131989 > 3rdparty/libprocess/configure.ac 1644035 > 3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION > > Diff: https://reviews.apache.org/r/52695/diff/ > > > Testing > ------- > > Compared the benchmarks with and without the flags being used. Also did a > comparsion with the flags being used with and without optimizations and > without the flags being used with and without optimizations. Overall the > performance hit was very small with a 3-8% overhead (optimizations brings > this down slightly). Most benchmarks were about 5% (or less) slower. > > > Thanks, > > Aaron Wood > >
