Skip to content

Recycle test runners more aggressively - #8751

Merged
sklam merged 1 commit into
numba:mainfrom
apmasell:mulitprocess_recycling
Feb 15, 2023
Merged

Recycle test runners more aggressively#8751
sklam merged 1 commit into
numba:mainfrom
apmasell:mulitprocess_recycling

Conversation

@apmasell

@apmasell apmasell commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

This changes the multiprocess pool to use a single long-lived pool with short-lived workers that are automatically restarted by the pool rather than operating in batches. This is to reduce idle time between tests.

@apmasell
Recycle test runners more aggressively
66e14e0 
This changes the multiprocess pool to use a single long-lived pool with
short-lived workers that are automatically restarted by the pool rather than
operating in batches. This is to reduce idle time between tests.
@gmarkall

gmarkall commented Feb 14, 2023

Copy link
Copy Markdown
Member

gpuci run tests

gmarkall
gmarkall reviewed Feb 14, 2023
View reviewed changes

@gmarkall gmarkall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tried this with the CUDA testsuite (python runtests.py numba.cuda.tests -m) and it didn't speed things up, but it didn't slow them down either. So it's OK from the CUDA side of things.

@apmasell

apmasell commented Feb 14, 2023

Copy link
Copy Markdown
Contributor Author

Okay. I assume the CUDA suite is smaller and bottlenecks on different resources, so I guess that isn't surprising.

@gmarkall

gmarkall commented Feb 14, 2023

Copy link
Copy Markdown
Member

I had hoped it would at least shave a couple of seconds off, because there are some tests that just compile a lot of things in parallel without touching the GPU. Never mind :-)

@sklam sklam self-assigned this Feb 14, 2023
@sklam

sklam commented Feb 14, 2023

Copy link
Copy Markdown
Member

I also didn't observe much speed difference, but the simplification of the code is nice.

@sklam

sklam commented Feb 14, 2023

Copy link
Copy Markdown
Member

I tested this again on a 160 core power8 machine and the result is so much more interesting. The command that i used: time python runtests.py -j0:10 -m -vb

On main c12bbf4:

----------------------------------------------------------------------
Ran 1037 tests in 295.618s

OK (skipped=85, expected failures=3)

real	5m0.635s
user	25m58.936s
sys	1m6.356s

On this PR branch:

----------------------------------------------------------------------
Ran 1037 tests in 220.317s

OK (skipped=85, expected failures=3)

real	3m43.361s
user	44m3.620s
sys	0m48.908s

There's a 1.3x improvement. Reduction in sys time is good indication of less syscall overhead. Not sure how to interpret the user time though.

@stuartarchibald

stuartarchibald commented Feb 15, 2023

Copy link
Copy Markdown
Contributor

Not sure how to interpret the user time though.

I'm not sure how to interpret this either. The tests that ran in -j 0:10 include some of stress tests for the threading backends which are deliberately left to do unpredictable things, could that be involved? Maybe a rerun would yield a different result?

Either way, the code simplification is worthwhile.

@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Feb 15, 2023
@sklam sklam added this to the Numba 0.57 RC milestone Feb 15, 2023
@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Feb 15, 2023
@sklam
sklam merged commit 386a8f9 into numba:main Feb 15, 2023
sklam added a commit to sklam/numba that referenced this pull request Feb 22, 2023
@sklam
Revert "Merge pull request numba#8751 from apmasell/mulitprocess_recy…
a746630 
…cling"

386a8f9
@sklam sklam mentioned this pull request Feb 22, 2023
Merged
esc added a commit to esc/numba that referenced this pull request Mar 2, 2023
@esc
Merge branch 'main' into fix_index_casting_in_typed_list_getitem
4ebfe6b 
* main: (2743 commits)
  Upgrade stale GitHub action
  Changed wording of the escape error
  Correct documentation for CAS
  Revert "Merge pull request numba#8751 from apmasell/mulitprocess_recycling"
  CUDA driver.py: PR numba#8180 feedback
  CUDA Docs: Refine MVC docs based on PR numba#8180 feedback
  revert build_ext change
  add empty line back
  revert some changes
  fix compiler arg, revert build script
  add build_doc back, fix compiler arg
  Correct grammar in comment
  fix import
  Stale bot: Use `abandoned - stale` label for closed PRs
  refactor build_ext
  BLD: remove distutils
  CUDA tidy-up: remove some unneeded methods
  Update numba/tests/support.py
  Update .github/workflows/stale.yml
  Fix line length error from flake8
  ...
@apmasell
apmasell deleted the mulitprocess_recycling branch March 13, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmarkall gmarkall gmarkall left review comments
@sklam sklam sklam approved these changes
@stuartarchibald stuartarchibald Awaiting requested review from stuartarchibald

Assignees

@sklam sklam

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed

Projects

None yet

Milestone

Numba 0.57 RC

Development

Successfully merging this pull request may close these issues.

4 participants

@apmasell @gmarkall @sklam @stuartarchibald