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

test_free_threading and test_super take way to long on a Free Threaded build. #124402

Closed
terryjreedy opened this issue Sep 24, 2024 · 16 comments
Closed
Labels
performance Performance or resource usage tests Tests in the Lib/test dir topic-free-threading type-bug An unexpected behavior, bug, or error

Comments

@terryjreedy
Copy link
Member

terryjreedy commented Sep 24, 2024

Bug report

Running freshly updated and compiled main on my new machine, python -m test -j0 (I forgot -ugui) ran all but 2 tests in 2:46. (16 cores defaulted to 2x16 + 2 == 34 processes.) The longest running was test_multiprocessing_spawn.test_processes at 2:18. But test_super took 10:13 and test_free_threading took 13:37. These both need to be broken up into one or more directories with multiple files in each, which each file running faster than the longest 'other' test. Several other tests, including multiprocessing have been broken up in the last year to allow testing to complete faster on multicore machines. (I don't know what label, if any, might be added for test_super.)

Linked PRs

@terryjreedy terryjreedy added type-bug An unexpected behavior, bug, or error performance Performance or resource usage tests Tests in the Lib/test dir topic-free-threading labels Sep 24, 2024
@colesbury
Copy link
Contributor

We should definitely make test_free_threading faster. I think it's mostly a matter of reducing the number of threads or the number of iterations in a few test cases.

@colesbury
Copy link
Contributor

@terryjreedy - were you testing with a build configured with --disable-gil? The test_free_threading tests should be skipped entirely on the default (with GIL) build.

@terryjreedy
Copy link
Member Author

Yes: PCbuild\build.bat -d --disable-gil. Called 'experimental free threading' when run. Non-debug times would be 2/3rds or better.

@terryjreedy
Copy link
Member Author

There is also requires('cpu') (spelling) for needed long-running tests. I an not sure where the list of legal requirements is.

@vstinner
Copy link
Member

vstinner commented Sep 24, 2024

test_super took 10:13

That's surprising. On Linux, on Python built in debug mode without any optimization (gcc -O0), it takes less than a second (581 ms). -- I'm talking about a regular Python build, not using --disable-gil.

vstinner added a commit to vstinner/cpython that referenced this issue Sep 24, 2024
test___class___modification_multithreaded() now requires the 'cpu'
tets resource on Free Threaded build.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 24, 2024
test___class___modification_multithreaded() now requires the 'cpu'
tets resource on Free Threaded build.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 24, 2024
test___class___modification_multithreaded() now requires the 'cpu'
test resource on a Free Threaded build.
@vstinner
Copy link
Member

That's surprising. On Linux, on Python built in debug mode without any optimization (gcc -O0), it takes less than a second (581 ms). -- I'm talking about a regular Python build, not using --disable-gil.

Oh ok, test_super takes a few minutes on a Free Threaded build: I created PR gh-124434 to skip the test unless the "cpu" test resource is enabled.

@vstinner vstinner changed the title test_free_threading and test_super take way to long. test_free_threading and test_super take way to long on a Free Threaded build. Sep 24, 2024
@vstinner
Copy link
Member

Timing on Free Threaded builds.

Linux: Python built in debug mode with gcc -O0 (without any optimization), laptop with 12 logicial CPUs.

  • test_super takes 40.6 seconds
  • test_free_threading takes 1 min 6 sec

Windows: Python built with PCbuild\build.bat -e -d -p x64 --disable-gil, VM with 6 logicial CPUs.

  • test_super takes 5 min 57 sec
  • test_free_threading takes 8 min 42 sec

vstinner added a commit to vstinner/cpython that referenced this issue Sep 24, 2024
Require the 'cpu' test resource on slow test_free_threading tests.
@vstinner
Copy link
Member

By the way, test_free_threading.test___class___modification() and test_super.test___class___modification_multithreaded() are the same test!

vstinner added a commit that referenced this issue Sep 24, 2024
Require the 'cpu' test resource on slow test_free_threading tests.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 24, 2024
…124438)

Require the 'cpu' test resource on slow test_free_threading tests.

(cherry picked from commit 38a5beb)
@Fidget-Spinner
Copy link
Member

By the way, test_free_threading.test___class___modification() and test_super.test___class___modification_multithreaded() are the same test!

Yup that was my mistake.

Yhg1s pushed a commit that referenced this issue Sep 24, 2024
… (#124439)

gh-124402: Require cpu resource in test_free_threading (#124438)

Require the 'cpu' test resource on slow test_free_threading tests.

(cherry picked from commit 38a5beb)
vstinner added a commit that referenced this issue Sep 24, 2024
test___class___modification_multithreaded() now requires the 'cpu'
test resource on a Free Threaded build.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 24, 2024
…onGH-124434)

test___class___modification_multithreaded() now requires the 'cpu'
test resource on a Free Threaded build.
(cherry picked from commit 5a60566)

Co-authored-by: Victor Stinner <[email protected]>
@vstinner
Copy link
Member

Yup that was my mistake.

Should we only keep the test in test_super? I'm not sure of the relationship between super() and the test.

@vstinner
Copy link
Member

@colesbury:

@vstinner - Please slow down with these changes. Marking these tests as cpu makes them much less useful because they do not run on PRs in GitHub CI.

Ok. At least, tests are still run on buildbots, and skipping slowest tests for now should make GitHub Actions a little bit faster.

@colesbury:

As I wrote on the original issue, we should be reducing the number of iterations and the number of threads so that they can run quickly.

Got it. Is there a risk of not testing the bug anymore if the number of iterations/threads is too low? How can we tune these without missing the point of the test?

Yhg1s pushed a commit that referenced this issue Sep 24, 2024
…124434) (#124468)

gh-124402: Require cpu resource in test_super slow method (GH-124434)

test___class___modification_multithreaded() now requires the 'cpu'
test resource on a Free Threaded build.
(cherry picked from commit 5a60566)

Co-authored-by: Victor Stinner <[email protected]>
@colesbury
Copy link
Contributor

Is there a risk of not testing the bug anymore if the number of iterations/threads is too low?

I think there's some risk, but it's usually worth the tradeoff of catching bugs earlier. I'm more concerned about missing bugs in buildbots -- it's easier to miss occasional failures if the tests aren't run as part of make test or the GitHub CI.

@terryjreedy
Copy link
Member Author

I consider the current fixes to be perhaps temporary. On the one hand, the two tests increased the CI sequential test run times perhaps 50% and with the sprint on, PR CIs are backing up and taking 1 1/2 or more hours. An immediate fix was really needed. On the other hand, I think test_free_threading should get the same minute that other tests are allowed, and some take. But working out priorities and adjusting tests may need some days.

vstinner added a commit to vstinner/cpython that referenced this issue Sep 25, 2024
Reduce the number of iterations and the number of threads so a whole
test file takes less than a minute.

Refactor test_racing_iter_extend() to remove two levels of
indentation.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 25, 2024
* Reduce the number of iterations and the number of threads so a
  whole test file takes less than a minute.
* Refactor test_racing_iter_extend() to remove two levels of
  indentation.
* test_monitoring() uses a sleep of 100 ms instead of 1 second.
@vstinner
Copy link
Member

I wrote a draft PR gh-124491 to remove @support.requires_resource('cpu') and still fit into a time budget of 1 minute per test file by reducing the number of iterations and threads.

corona10 pushed a commit to corona10/cpython that referenced this issue Sep 25, 2024
* Reduce the number of iterations and the number of threads so a
  whole test file takes less than a minute.
* Refactor test_racing_iter_extend() to remove two levels of
  indentation.
* test_monitoring() uses a sleep of 100 ms instead of 1 second.
vstinner added a commit that referenced this issue Sep 26, 2024
* Reduce the number of iterations and the number of threads so a
  whole test file takes less than a minute.
* Refactor test_racing_iter_extend() to remove two levels of
  indentation.
* test_monitoring() uses a sleep of 100 ms instead of 1 second.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 26, 2024
…124491)

* Reduce the number of iterations and the number of threads so a
  whole test file takes less than a minute.
* Refactor test_racing_iter_extend() to remove two levels of
  indentation.
* test_monitoring() uses a sleep of 100 ms instead of 1 second.

(cherry picked from commit 0387c34)
Yhg1s pushed a commit that referenced this issue Sep 26, 2024
… (#124585)

gh-124402: Speed up test_free_threading and test_super (#124491)

* Reduce the number of iterations and the number of threads so a
  whole test file takes less than a minute.
* Refactor test_racing_iter_extend() to remove two levels of
  indentation.
* test_monitoring() uses a sleep of 100 ms instead of 1 second.

(cherry picked from commit 0387c34)
@terryjreedy
Copy link
Member Author

Any more planned PRs, or close?

@vstinner
Copy link
Member

We're done, I closed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage tests Tests in the Lib/test dir topic-free-threading type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants