You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2018/05/14 09:18:26 UTC

Review Request 67108: Did not omit frame pointers in debug builds.

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

Review request for mesos.


Repository: mesos


Description
-------

This patch prevents the compiler from omitting frame pointers for
debug builds which can lead to a better debugging or profiling
experience.

Since having this flag turned on can disable certain optimizations we
do not enable it in non-debug builds. This can lead to minor
deviations in runtime behavior when combing release and debug flags
(e.g., when combining `--enable-debug` with `--enable-optimize`, or
with the cmake build type `RelWithDebInfo`), but still should give the
best possible build for pure release builds, and allow a reasonable
developer experience when debugging/profiling.


Diffs
-----

  cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
  configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 


Diff: https://reviews.apache.org/r/67108/diff/1/


Testing
-------


Thanks,

Benjamin Bannier


Re: Review Request 67108: Disabled omission of frame pointers in debug builds.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67108/
-----------------------------------------------------------

(Updated May 14, 2018, 8:38 p.m.)


Review request for mesos, Benjamin Mahler and James Peach.


Summary (updated)
-----------------

Disabled omission of frame pointers in debug builds.


Bugs: MESOS-8908
    https://issues.apache.org/jira/browse/MESOS-8908


Repository: mesos


Description
-------

This patch prevents the compiler from omitting frame pointers for
debug builds which can lead to a better debugging or profiling
experience.

Since having this flag turned on can disable certain optimizations we
do not enable it in non-debug builds. This can lead to minor
deviations in runtime behavior when combing release and debug flags
(e.g., when combining `--enable-debug` with `--enable-optimize`, or
with the cmake build type `RelWithDebInfo`), but still should give the
best possible build for pure release builds, and allow a reasonable
developer experience when debugging/profiling.


Diffs
-----

  cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
  configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 


Diff: https://reviews.apache.org/r/67108/diff/1/


Testing
-------

`make check`


Thanks,

Benjamin Bannier


Re: Review Request 67108: Did not omit frame pointers in debug builds.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On May 14, 2018, 5:14 p.m., James Peach wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 325 (patched)
> > <https://reviews.apache.org/r/67108/diff/1/?file=2021184#file2021184line325>
> >
> >     `RelWithDebInfo` is still a release build, right? That seems counter to your intention here.

My intention was to add the flag to all builds emitting debug information so we can report complete stack traces there.

This does include `RelWithDebInfo` since somebody conciously decided to include debug info along with enabling optimizations (e.g., for production-like performance measurements with `perf`).

Dropping this, please reopen if I missed your point.


- Benjamin


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


On May 14, 2018, 11:18 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67108/
> -----------------------------------------------------------
> 
> (Updated May 14, 2018, 11:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-8908
>     https://issues.apache.org/jira/browse/MESOS-8908
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch prevents the compiler from omitting frame pointers for
> debug builds which can lead to a better debugging or profiling
> experience.
> 
> Since having this flag turned on can disable certain optimizations we
> do not enable it in non-debug builds. This can lead to minor
> deviations in runtime behavior when combing release and debug flags
> (e.g., when combining `--enable-debug` with `--enable-optimize`, or
> with the cmake build type `RelWithDebInfo`), but still should give the
> best possible build for pure release builds, and allow a reasonable
> developer experience when debugging/profiling.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
>   configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 
> 
> 
> Diff: https://reviews.apache.org/r/67108/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 67108: Did not omit frame pointers in debug builds.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On May 14, 2018, 5:14 p.m., James Peach wrote:
> > Nit: I'd title the subject "Do not omit ..."
> 
> Andrew Schwartzmeyer wrote:
>     I would too; but Mesos really likes past tense titles. /shrug

Bear with me, this is a non-native speaker attempting to juggle multiple constraints (past tense, 72 char limit, clear and explicit, end in a period).

I'd vote for keeping the current form unless you guys can come up with something that works as well ... let's not set controversial precedents for no good reason :D


- Benjamin


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


On May 14, 2018, 11:18 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67108/
> -----------------------------------------------------------
> 
> (Updated May 14, 2018, 11:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-8908
>     https://issues.apache.org/jira/browse/MESOS-8908
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch prevents the compiler from omitting frame pointers for
> debug builds which can lead to a better debugging or profiling
> experience.
> 
> Since having this flag turned on can disable certain optimizations we
> do not enable it in non-debug builds. This can lead to minor
> deviations in runtime behavior when combing release and debug flags
> (e.g., when combining `--enable-debug` with `--enable-optimize`, or
> with the cmake build type `RelWithDebInfo`), but still should give the
> best possible build for pure release builds, and allow a reasonable
> developer experience when debugging/profiling.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
>   configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 
> 
> 
> Diff: https://reviews.apache.org/r/67108/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 67108: Did not omit frame pointers in debug builds.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

> On May 14, 2018, 8:14 a.m., James Peach wrote:
> > Nit: I'd title the subject "Do not omit ..."

I would too; but Mesos really likes past tense titles. /shrug


- Andrew


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


On May 14, 2018, 2:18 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67108/
> -----------------------------------------------------------
> 
> (Updated May 14, 2018, 2:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-8908
>     https://issues.apache.org/jira/browse/MESOS-8908
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch prevents the compiler from omitting frame pointers for
> debug builds which can lead to a better debugging or profiling
> experience.
> 
> Since having this flag turned on can disable certain optimizations we
> do not enable it in non-debug builds. This can lead to minor
> deviations in runtime behavior when combing release and debug flags
> (e.g., when combining `--enable-debug` with `--enable-optimize`, or
> with the cmake build type `RelWithDebInfo`), but still should give the
> best possible build for pure release builds, and allow a reasonable
> developer experience when debugging/profiling.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
>   configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 
> 
> 
> Diff: https://reviews.apache.org/r/67108/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 67108: Did not omit frame pointers in debug builds.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67108/#review203029
-----------------------------------------------------------


Fix it, then Ship it!




Nit: I'd title the subject "Do not omit ..."


cmake/CompilationConfigure.cmake
Lines 325 (patched)
<https://reviews.apache.org/r/67108/#comment285092>

    `RelWithDebInfo` is still a release build, right? That seems counter to your intention here.


- James Peach


On May 14, 2018, 9:18 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67108/
> -----------------------------------------------------------
> 
> (Updated May 14, 2018, 9:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-8908
>     https://issues.apache.org/jira/browse/MESOS-8908
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch prevents the compiler from omitting frame pointers for
> debug builds which can lead to a better debugging or profiling
> experience.
> 
> Since having this flag turned on can disable certain optimizations we
> do not enable it in non-debug builds. This can lead to minor
> deviations in runtime behavior when combing release and debug flags
> (e.g., when combining `--enable-debug` with `--enable-optimize`, or
> with the cmake build type `RelWithDebInfo`), but still should give the
> best possible build for pure release builds, and allow a reasonable
> developer experience when debugging/profiling.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 843786e2ff72cd1a07be73d97a071f0f1398354a 
>   configure.ac 6d9c8ceb083984b9b61b30c29c39b3aef32fbd69 
> 
> 
> Diff: https://reviews.apache.org/r/67108/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>