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

Speed up gazebo test compilation #955

Closed
osrf-migration opened this issue Nov 16, 2013 · 13 comments
Closed

Speed up gazebo test compilation #955

osrf-migration opened this issue Nov 16, 2013 · 13 comments
Labels

Comments

@osrf-migration
Copy link

Original report (archived issue) by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Currently several source files are recompiled for every test (Server.cc, Master.cc, gazebo.cc, and ServerFixture.cc). We could probably make a single static library from these files to speed up compilation and reduce the size of the build folder (the object files corresponding to these 4 source files take up about 350 MB in my build folder).

See the GZ_BUILD_TESTS_EXTRA_EXE_SRCS variable in test/integration/CMakeLists.txt (and a few other places) and the gz_build_tests macro in cmake/GazeboTestUtils.cmake

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Currently testing the following:

diff -r e7974eec6beb67c2da10087c8b341d3ca8212fb2 cmake/GazeboTestUtils.cmake
--- a/cmake/GazeboTestUtils.cmake	Fri Nov 15 00:07:39 2013 -0800
+++ b/cmake/GazeboTestUtils.cmake	Fri Nov 15 23:46:18 2013 -0800
@@ -10,7 +10,8 @@
     if(USE_LOW_MEMORY_TESTS)
       add_definitions(-DUSE_LOW_MEMORY_TESTS=1)
     endif(USE_LOW_MEMORY_TESTS)
-    add_executable(${BINARY_NAME} ${GTEST_SOURCE_file} ${GZ_BUILD_TESTS_EXTRA_EXE_SRCS})
+    #add_executable(${BINARY_NAME} ${GTEST_SOURCE_file} ${GZ_BUILD_TESTS_EXTRA_EXE_SRCS})
+    add_executable(${BINARY_NAME} ${GTEST_SOURCE_file})
 
     add_dependencies(${BINARY_NAME}
       gtest gtest_main
@@ -21,12 +22,14 @@
       gazebo_rendering
       gazebo_msgs
       gazebo_transport
+      server_fixture
       )
 
 
     target_link_libraries(${BINARY_NAME}
       libgtest.a
       libgtest_main.a
+      libserver_fixture.a
       gazebo_common
       gazebo_math
       gazebo_physics
diff -r e7974eec6beb67c2da10087c8b341d3ca8212fb2 test/CMakeLists.txt
--- a/test/CMakeLists.txt	Fri Nov 15 00:07:39 2013 -0800
+++ b/test/CMakeLists.txt	Fri Nov 15 23:46:18 2013 -0800
@@ -29,6 +29,16 @@
 set(GTEST_LIBRARY "${PROJECT_BINARY_DIR}/test/libgtest.a")
 set(GTEST_MAIN_LIBRARY "${PROJECT_BINARY_DIR}/test/libgtest_main.a")
 
+# Build static ServerFixture library
+set(ServerFixtureSources
+  ${PROJECT_SOURCE_DIR}/gazebo/Server.cc
+  ${PROJECT_SOURCE_DIR}/gazebo/Master.cc
+  ${PROJECT_SOURCE_DIR}/gazebo/gazebo.cc
+  ${PROJECT_SOURCE_DIR}/test/ServerFixture.cc
+)
+add_library(server_fixture STATIC ${ServerFixtureSources})
+set(SERVER_FIXTURE_LIBRARY "${PROJECT_BINARY_DIR}/test/libserver_fixture.a")
+
 execute_process(COMMAND cmake -E remove_directory ${CMAKE_BINARY_DIR}/test_results)
 execute_process(COMMAND cmake -E make_directory ${CMAKE_BINARY_DIR}/test_results)
 include_directories(${GTEST_INCLUDE_DIRS})

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


  • changed priority from "major" to "minor"
  • set assignee to "scpeters (Bitbucket: scpeters, GitHub: scpeters)"
  • set assignee_account_id to "557058:5de38267-b118-494c-aa76-4fab35448816"

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


fe5678c in branch issue_955

It appears to speed up compilation by 20-25% and reduce build folder size by 200 MB for RelWithDebInfo

EDIT: these tests were done on osx without ccache

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I did some testing with ccache on quantal, and the speed gain is much lower (about 9% faster), though I would still expect a more significant speed-up without ccache. The folder size was still reduced by about 200 MB.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I did some further refactoring of ServerFixture, moving the code out of the header file and into ServerFixture.cc. This gives even greater speed-ups, leading to a total speed-up of 30% and a 500 MB reduction in build folder size.

Timing info on quantal with ccache enabled:

Reference build, gazebo_2.1 branch, make -j4

$ time make -j4 && du -hs . && hg branch
real	12m38.220s
user	47m7.221s
sys	2m13.696s
2.7G	.
gazebo_2.1

9% speed-up with issue_955 branch

$ time make -j4 && du -hs . && hg branch
real	11m42.127s
user	43m23.879s
sys	2m4.920s
2.4G	.
issue_955

30% speed-up with issue_955_refactor branch

real	8m53.228s
user	32m41.979s
sys	1m43.094s
2.2G	.
issue_955_refactor

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


The diff for issue_955_refactor is pretty nasty, maybe rollback some of the #include visibility stinginess for another time.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


part 1 in pull request #846

@osrf-migration
Copy link
Author

Original comment by Jose Luis Rivero (Bitbucket: Jose Luis Rivero, GitHub: j-rivero).


Impresive!

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Unnecessary linking was reduced in pull request #2130, which allows many unit tests to be compiled more quickly, for example:

mkdir build
cd build
cmake ..
cd gazebo/math
make -j8
make test

This no longer requires compiling server_fixture, so it goes much faster.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I'm going to mark this as resolved. There were some improvements that reduced unnecessary includes, but I'll leave that for another day and another issue.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


  • changed state from "new" to "resolved"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • set version to "all"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "resolved" to "closed"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant