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

make check compilation is unneccesarily slow #445

Closed
horenmar opened this issue Feb 3, 2017 · 7 comments
Closed

make check compilation is unneccesarily slow #445

horenmar opened this issue Feb 3, 2017 · 7 comments

Comments

@horenmar
Copy link

horenmar commented Feb 3, 2017

I recently used this project's test suite for some performance measurement of Catch and found out couple of things

  1. We have achieved significant run-time speedup
  2. We have had some compile-time slowdown
  3. Compilation of the json for modern c++ test suite takes a looong time

When I was investigating 2., I found out that when I run make check, the test suite is compiled into several distinct test binaries and each of those binaries is then ran. The proverbial devil is in the fact that for each of those 33 test binaries, the entire Catch main is compiled again for each of those, instead of having the Catch's main compiled once and then linked into each of the test binaries.

If I change the makefile so that the catch main is compiled once and only lnked into each test binary, the same PC that needed 16 minutes to compile the test binaries from nothing needs only 7 minutes, 45 s.

The relevant part of makefiles is in test/Makefile:

test-%: src/unit-%.cpp ../src/json.hpp thirdparty/catch/catch.hpp
	@echo "[CXXLD] $@"
	@$(CXX) $(CXXFLAGS) $(CPPFLAGS) $(LDFLAGS) -DCATCH_CONFIG_MAIN $< -o $@

if the -DCATCH_CONFIG_MAIN was replaced by src/unit.o (and the file was also added to dependencies), the compilation should still work and be significantly faster.

@nlohmann
Copy link
Owner

nlohmann commented Feb 3, 2017

Wow, I can confirm a 2x improvement:

before

  • make 118.56s user 5.26s system 99% cpu 2:04.76 total
  • make -j8 192.52s user 7.73s system 677% cpu 29.554 total

after:

  • make 58.13s user 3.00s system 99% cpu 1:01.75 total
  • make -j8 95.88s user 4.50s system 696% cpu 14.417 total

Thanks so much!

@nlohmann nlohmann self-assigned this Feb 3, 2017
@nlohmann nlohmann added this to the Release 2.1.1 milestone Feb 3, 2017
nlohmann added a commit that referenced this issue Feb 3, 2017
@horenmar
Copy link
Author

horenmar commented Feb 3, 2017

I'd also recommend pulling in Catch 1.7.0, I got about a 30% speed up on actual runtime of the test suite.

@nlohmann
Copy link
Owner

nlohmann commented Feb 3, 2017

@horenmar I just realized that commit 5976caf did improve the compilation times, but no tests are executed...

Executing test-iterator_wrapper...
===============================================================================
No tests ran

Any idea what I did wrong?

@horenmar
Copy link
Author

horenmar commented Feb 3, 2017

You added dependency on unit.o, but did not add it to compilation command.
This is what it should look like: @$(CXX) $(CXXFLAGS) $(CPPFLAGS) $(LDFLAGS) src/unit.o $< -o $@, you are missing src/unit.o

@nlohmann
Copy link
Owner

nlohmann commented Feb 3, 2017

Well... This is embarrassing... Thank you! I shall also try Catch 1.7.0.

@nlohmann
Copy link
Owner

nlohmann commented Feb 4, 2017

Unfortunately, Catch 1.7.0 yields a lot of compilation errors

src/unit-allocator.cpp:31:9: warning: keyword is hidden by macro definition [-Wkeyword-macro]
#define private public
        ^
src/unit-allocator.cpp:62:25: error: unexpected type name 'bad_json': expected expression
        CHECK_THROWS_AS(bad_json j(bad_json::value_t::object), std::bad_alloc);
                        ^
src/unit-allocator.cpp:62:34: error: expected ')'
        CHECK_THROWS_AS(bad_json j(bad_json::value_t::object), std::bad_alloc);
                                 ^
src/unit-allocator.cpp:62:9: note: to match this '('
        CHECK_THROWS_AS(bad_json j(bad_json::value_t::object), std::bad_alloc);
        ^
thirdparty/catch/catch.hpp:10946:48: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                               ^
thirdparty/catch/catch.hpp:2283:34: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                 ^
src/unit-allocator.cpp:148:51: error: expected '(' for function-style cast or type construction
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                              ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10948:56: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                                                       ^~~~
thirdparty/catch/catch.hpp:2251:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                              ^~~~
src/unit-allocator.cpp:148:51: error: expected ')'
src/unit-allocator.cpp:148:17: note: to match this '('
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:30: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                             ^
src/unit-allocator.cpp:148:17: error: extraneous ')' before ';'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:35: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                                  ^
src/unit-allocator.cpp:148:66: error: use of undeclared identifier 'j'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                                                                 ^
src/unit-allocator.cpp:150:53: error: expected '(' for function-style cast or type construction
                CHECK_THROWS_AS(my_json::json_value j(t), std::bad_alloc);
                                ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10946:74: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                                                         ^~~~
thirdparty/catch/catch.hpp:2283:35: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                  ^~~~
src/unit-allocator.cpp:150:53: error: expected ')'
src/unit-allocator.cpp:150:17: note: to match this '('
                CHECK_THROWS_AS(my_json::json_value j(t), std::bad_alloc);
                ^
thirdparty/catch/catch.hpp:10946:48: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                               ^
thirdparty/catch/catch.hpp:2283:34: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                 ^
src/unit-allocator.cpp:161:51: error: expected '(' for function-style cast or type construction
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                              ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10948:56: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                                                       ^~~~
thirdparty/catch/catch.hpp:2251:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                              ^~~~
src/unit-allocator.cpp:161:51: error: expected ')'
src/unit-allocator.cpp:161:17: note: to match this '('
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:30: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                             ^
src/unit-allocator.cpp:161:17: error: extraneous ')' before ';'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:35: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                                  ^
src/unit-allocator.cpp:161:66: error: use of undeclared identifier 'j'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                                                                 ^
src/unit-allocator.cpp:163:53: error: expected '(' for function-style cast or type construction
                CHECK_THROWS_AS(my_json::json_value j(t), std::bad_alloc);
                                ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10946:74: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                                                         ^~~~
thirdparty/catch/catch.hpp:2283:35: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                  ^~~~
src/unit-allocator.cpp:163:53: error: expected ')'
src/unit-allocator.cpp:163:17: note: to match this '('
                CHECK_THROWS_AS(my_json::json_value j(t), std::bad_alloc);
                ^
thirdparty/catch/catch.hpp:10946:48: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                               ^
thirdparty/catch/catch.hpp:2283:34: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                 ^
src/unit-allocator.cpp:174:51: error: expected '(' for function-style cast or type construction
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                              ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10948:56: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                                                       ^~~~
thirdparty/catch/catch.hpp:2251:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                              ^~~~
src/unit-allocator.cpp:174:51: error: expected ')'
src/unit-allocator.cpp:174:17: note: to match this '('
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:30: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                             ^
src/unit-allocator.cpp:174:17: error: extraneous ')' before ';'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                ^
thirdparty/catch/catch.hpp:10948:31: note: expanded from macro 'CHECK_NOTHROW'
#define CHECK_NOTHROW( expr ) INTERNAL_CATCH_NO_THROW( expr, Catch::ResultDisposition::ContinueOnFailure, "CHECK_NOTHROW" )
                              ^
thirdparty/catch/catch.hpp:2251:35: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
            static_cast<void>(expr); \
                                  ^
src/unit-allocator.cpp:174:66: error: use of undeclared identifier 'j'
                CHECK_NOTHROW(my_json::json_value j(t); clean_up(j));
                                                                 ^
src/unit-allocator.cpp:176:53: error: expected '(' for function-style cast or type construction
                CHECK_THROWS_AS(my_json::json_value j(t), std::bad_alloc);
                                ~~~~~~~~~~~~~~~~~~~ ^
thirdparty/catch/catch.hpp:10946:74: note: expanded from macro 'CHECK_THROWS_AS'
#define CHECK_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::ContinueOnFailure, "CHECK_THROWS_AS" )
                                                                         ^~~~
thirdparty/catch/catch.hpp:2283:35: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                  ^~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]

@nlohmann
Copy link
Owner

nlohmann commented Feb 4, 2017

I opened an issue at Catch. The job here is done. Thanks a lot!

@nlohmann nlohmann closed this as completed Feb 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants