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

Shorten the time to verify test discovery. #8427

Merged
merged 2 commits into from Sep 14, 2022

Conversation

stuartarchibald
Copy link
Contributor

The Numba project uses python -m numba.tests.test_runtests in build scripts to verify that test discovery and the test suite itself is working as expected. This is a guard to make sure that when the test suite is run, it's actually running what is expected.

This patch tries to shorten the time it takes to get to the point of the test suite being validated. It does two things:

  1. Hoists a costly loop invariant from a loop in one of the tests in the test_runtests module.
  2. Caches the mtime of test files based on class as this should be invariant (gains about 3-4% improvement on runtests.py -l).

The Numba project uses `python -m numba.tests.test_runtests`
in build scripts to verify that test discovery and the test suite
itself is working as expected. This is a guard to make sure that
when the test suite is run, it's actually running what is expected.

This patch tries to shorten the time it takes to get to the point
of the test suite being validated. It does two things:

1. Hoists a costly loop invariant from a loop in one of the tests
   in the test_runtests module.
2. Caches the mtime of test files based on class as this should be
   invariant (gains about 3-4% improvement on runtests.py -l).
@guilhermeleobas guilhermeleobas marked this pull request as ready for review September 13, 2022 14:31
@stuartarchibald stuartarchibald added this to the Numba 0.57 RC milestone Sep 13, 2022
@stuartarchibald stuartarchibald added 4 - Waiting on second reviewer Patch needs a second reviewer. and removed 3 - Ready for Review labels Sep 13, 2022
Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

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

I did a crude benchmark of this branch with main.

For main I get the following times: 70.326s and 69.016s and 64.027s.

For the feature branch I get: 62.353s and 54.295s and 55.061s.

This is sufficient evidence for me regarding the effect this change has, thank you for the patch!

@esc esc added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on second reviewer Patch needs a second reviewer. labels Sep 14, 2022
@sklam sklam merged commit f354f9a into numba:main Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants