Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add USE_PROFILER to Cmake #1119

Merged
merged 3 commits into from
Jul 30, 2018
Merged

add USE_PROFILER to Cmake #1119

merged 3 commits into from
Jul 30, 2018

Conversation

oxarbitrage
Copy link
Member

@oxarbitrage oxarbitrage commented Jul 5, 2018

Core developer @abitmore had been using sucessfully some profiling with gprof in our binaries at different scenarios for example #1113

this pull adds the option to compile with profile information by cmake, something as:

cmake -DBOOST_ROOT="$BOOST_ROOT" -DCMAKE_BUILD_TYPE=RelWithDebInfo -DUSE_PROFILER=ON .

This option is off by default as the binary will be much more slower when adding profiling info. The use of gprof is outside of the scope, option should be used by an experienced developer interested in project performance.

A note written in #1140:

  • need cmake output msg that build will be made with profile.

@abitmore
Copy link
Member

abitmore commented Jul 5, 2018

Thanks.
My local setup has this, I'm not sure if it's necessary though:

         if ( FULL_STATIC_BUILD )
-          set( CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libstdc++ -static-libgcc")
+          set( CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libstdc++ -static-libgcc -pg")
         endif ( FULL_STATIC_BUILD )

@oxarbitrage
Copy link
Member Author

i actually had that too in my initial setup but then confirmed it is not needed, the gmon.out is generated without it and gprof just works fine.

Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • IMO this will get messy if more such options are added in the future. Better keep the original line and only add "-pg" if USE_PROFILER is ON.
  • Perhaps add -pg to CMAKE_C_FLAGS not only CMAKE_CXX_FLAGS?

@oxarbitrage
Copy link
Member Author

IMO this will get messy if more such options are added in the future. Better keep the original line and only add "-pg" if USE_PROFILER is ON.

changed this.

Perhaps add -pg to CMAKE_C_FLAGS not only CMAKE_CXX_FLAGS?

I made some tests with this and also by adding pg to the linker flags before. I am not an expert in profiling, actually my first time checking this out but i think they don't make any difference.

pmconrad
pmconrad previously approved these changes Jul 25, 2018
@abitmore abitmore dismissed pmconrad’s stale review July 27, 2018 13:35

Still things to be done

@oxarbitrage oxarbitrage merged commit 583a06b into develop Jul 30, 2018
@abitmore abitmore deleted the oxarbitrage-patch-4 branch July 30, 2018 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants