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

Testsuite: remove implicit target dependencies when possible (ninja support) #16555

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Jan 27, 2024

This is a first part in a series of testsuite performance improvements.

Remove unnecessary, implicit target dependencies in our testsuite between for tests with shared executable targets. These are all test variants where multiple tests share a single executable target, such as, for example MPI tests:

foobar.cc
foobar.mpirun=2.output
foobar.mpirun=4.output

Here, we ensure that an executable foobar is built from the foobar.cc sources before running either of the two test variants. This happens by first creating an executable target for foobar and then creating a test test_dependency/category.foobar.(debug|release).executable and require it to run before the category/foobar.(debug|release) with the help of CTEST's FIXTURES_REQUIRED mechanism.

We need this in order to support ninja for the testsuite: concurrent invocation of ninja (as we do in the testsuite) can lead to an incomplete .ninja_log file so that subsequent invocations do not rebuild the executable (which leads to unnecessary compilation times, and worse, race conditions).

  • CMake: split implicit target dependencies for shared executable dependencies
  • CMake: chain CMAKE_GENERATOR through to test subprojects

@tamiko
Copy link
Member Author

tamiko commented Jan 27, 2024

I want to run the full testsuite with ninja on this to verify that nothing bad happens.

@tamiko
Copy link
Member Author

tamiko commented Jan 27, 2024

Some timings: With make:

% time make -j36 setup_tests
310.90s user 306.03s system 1456% cpu 42.356 total

% ctest -R "base/aligned_vector_01" -j 4
1/2 Test #265: base/aligned_vector_01.release ...   Passed    7.63 sec
2/2 Test #264: base/aligned_vector_01.debug .....   Passed    7.81 sec
Total Test time (real) =   9.32 sec

% ctest -R "base/aligned_vector_01" -j 4
1/2 Test #264: base/aligned_vector_01.debug .....   Passed    0.85 sec
2/2 Test #265: base/aligned_vector_01.release ...   Passed    0.87 sec
Total Test time (real) =   2.40 sec

With Ninja:

% time ninja setup_tests
303.96s user 294.81s system 1567% cpu 38.195 total

% ctest -R "base/aligned_vector_01" -j 4
1/2 Test #265: base/aligned_vector_01.release ...   Passed    7.03 sec
2/2 Test #264: base/aligned_vector_01.debug .....   Passed    7.17 sec
Total Test time (real) =   8.72 sec

% ctest -R "base/aligned_vector_01" -j 4
1/2 Test #264: base/aligned_vector_01.debug .....   Passed    0.24 sec
2/2 Test #265: base/aligned_vector_01.release ...   Passed    0.23 sec
Total Test time (real) =   1.80 sec

Or in total runtime:

Make:  ctest -j 18 -R "base/"  7302.02s user 545.13s system 1665% cpu 7:51.21 total
Ninja: ctest -j 18 -R "base/"  6894.51s user 493.30s system 1673% cpu 7:21.42 total

... a saving of 6% runtime/cputime. Or a total of 400s CPU time which amounts to about 1.5h of CPU time saving for the whole testsuite.

…dencies

This commit ensures that deal_ii_add_test() will always unlink implicit
target dependencies when configuring a "shared target" that splits (a)
compilation and linkage of the executable and (b) run and comparison
into two separate tests.
@tamiko
Copy link
Member Author

tamiko commented Jan 29, 2024

All in all this saves a little bit of execution time. I will try to sneak in a quick test on the cluster today to see whether this breaks anything.

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

Successfully merging this pull request may close these issues.

None yet

4 participants