Skip to content

Performance issues when using oversubscription #10426

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

Closed
dalcinl opened this issue May 24, 2022 · 16 comments
Closed

Performance issues when using oversubscription #10426

dalcinl opened this issue May 24, 2022 · 16 comments

Comments

@dalcinl
Copy link
Contributor

dalcinl commented May 24, 2022

I've been testing mpi4py against ompi/main using an automatic schedule that runs on a weekly basis on GitHub Actions.

The GitHub-hosted runners have only two virtual cores, but I run tests with up to 5 MPI processes turning on oversubscription (setting rmaps_default_mapping_policy = :oversubscribe within $HOME/.prte/mca-params.conf).
The current workflow file is here.

Up to January 29, the testsuite used to take around 4m 30s to run to completion [link].
Since February 5, the same testsuite needs around 1h 20m to finish [link].
Unfortunately, the actual logs are long gone, but at least you can see the elapsed time (look for the Test mpi4py (np=5) line).

Is this a regression or some expected change in behavior?
Is there a new configuration option I'm missing that would allow me to go back to the previous behavior?

@dalcinl dalcinl changed the title Performance issues when using oversuscription Performance issues when using oversubscription May 24, 2022
@jsquyres jsquyres added this to the v5.0.0 milestone May 24, 2022
@jsquyres
Copy link
Member

@awlauria Can you answer @dalcinl's question? If there's a config change that is needed, this definitely needs to be documented for v5.0.x.

@devreal
Copy link
Contributor

devreal commented May 24, 2022

@dalcinl I don't remember any changes this year that could cause this but I had a patch a while back that added an option to control how MPI processes yield the CPU when idle. The default is still sched_yield but you can set --mca threads_pthreads_yield_strategy nanosleep to use the nanosleep call instead (and --mca threads_pthreads_nanosleep_time X to control the time slept; default is the minimum of a nanosecond, just to trigger the scheduler). This seems to do a better job at forcing processes to be rescheduled. Maybe that will help bring down your run times when oversubscribing. It shouldn't have any negative performance impact if not oversubscribing.

@awlauria
Copy link
Contributor

I am also not aware of anything that went in that would cause such a regression in performance. This seems alarming though, and something that should be tracked down.

@dalcinl
Copy link
Contributor Author

dalcinl commented May 25, 2022

@devreal My main concern is that I made absolutely no change to the way I configure Open MPI (either compile time or runtime), and yet I get this huge performance regression. Perhaps it is related to some change in GitHub Actions and how the runners are setup or configured. In the mean time, I'll try your suggestion and report back on the outcome.

@dalcinl
Copy link
Contributor Author

dalcinl commented May 25, 2022

@devreal I tried your suggestion (with the default nanosleep time), but had no effect.

@dalcinl
Copy link
Contributor Author

dalcinl commented May 25, 2022

@devreal @awlauria After manually bisecting the issue 😓, I can tell for sure the regression is within Open MPI.

The last good commit is 3b4d64f [logs], running mpi4py testsuite on 5 MPI processes takes under 3 minutes.

The regression is introduced after merging #9097 (MPI Sessions PR) in commit 7291361 [logs] (cancelled after 9 minutes of running on 3 MPI processes).

@hppritcha From your recent interactions in other issues I submitted, I believe you may want to get involved in this one.

@devreal
Copy link
Contributor

devreal commented May 25, 2022

Thanks for checking. Let me see if I can reproduce that on my machine

@hppritcha hppritcha self-assigned this May 25, 2022
@hppritcha
Copy link
Member

looking in to this.

@hppritcha
Copy link
Member

looks like a piece from 2b335ed disappeared with the sessions support

hppritcha added a commit to hppritcha/ompi that referenced this issue May 25, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
@hppritcha
Copy link
Member

@dalcinl could you try the patch at #10428 ?

You should see a message like this:

turning on yieldy thingy
turning on yieldy thingy
turning on yieldy thingy
turning on yieldy thingy
turning on yieldy thingy

if its working for your oversubscription case.

@dalcinl
Copy link
Contributor Author

dalcinl commented May 26, 2022

@devreal Look like your suggestion was not enough to workaround the issue. I'm just trying to figure out why.
Did you forget to mention that I should have set the MCA parameter mpi_yield_when_idle = true as well? Perhaps you assumed that yield_when_idle was already true, given that I was oversubscribing, and you did not know about the bug @hppritcha is fixing in #10428. Is either that, or the nanonsleep-based yield misbehaves or is broken, or the default time is not enough. Am I missing something?

@devreal
Copy link
Contributor

devreal commented May 26, 2022

Yes, I was assuming that this was set automatically but it looks like it isn't. Have you tried setting mpi_yield_when_idle = true manually?

@hppritcha
Copy link
Member

@dalcinl so does the patch in #10428 work or not? I was using the prte param you suggested to get ompi to add a yield in the opal progress loop.

@hppritcha
Copy link
Member

i think the intent of 2b335ed was to avoid forcing the user to set additional ompi mca parameters if prte knows that the ompi app is being run in an oversubscribed manner.

@dalcinl
Copy link
Contributor Author

dalcinl commented May 26, 2022

@hppritcha As I commented in #10428, your patch worked fine. My question to Joseph was unrelated to your fix, I just wanted to know why his suggestion did not work.

hppritcha added a commit to hppritcha/ompi that referenced this issue May 26, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
hppritcha added a commit to hppritcha/ompi that referenced this issue May 27, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
(cherry picked from commit f4156d3)
@awlauria
Copy link
Contributor

awlauria commented Jun 2, 2022

Merged to main and v5.0.x - closing.

@awlauria awlauria closed this as completed Jun 2, 2022
joaobfernandes0 pushed a commit to joaobfernandes0/ompi that referenced this issue Jun 17, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
awlauria pushed a commit to awlauria/ompi that referenced this issue Jun 22, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
wckzhang pushed a commit to wckzhang/ompi that referenced this issue Jun 30, 2022
The sessions related commit 7291361 inadvertenly removed a bit of commit 2b335ed.
Put it back in.

Leave a chatty string to help with testing.  this will be removed before merging.

Related to issue open-mpi#10426

Signed-off-by: Howard Pritchard <[email protected]>
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

5 participants