Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Prevent after-fork number of OMP threads being bigger than 1. #16999

Merged
merged 3 commits into from
Dec 11, 2019

Conversation

larroy
Copy link
Contributor

@larroy larroy commented Dec 7, 2019

Description

This could happen if it was set in the environment. As we are setting engine::OpenMP::Get()->set_enabled(false) in initialize.cc in the child after forking, the behaviour goes back to what it was before #15762 was introduced.

Regions using omp get the threads count from GetRecommendedOMPThreadCount, so if omp is disabled they will get 1 thread and run serially

@anirudh2290 @apeforest

This could happen if it was set in the environment. As we are setting engine::OpenMP::Get()->set_enabled(false) in initialize.cc in the child after forking, the behaviour goes back to what it was before apache#15762 was introduced.

Regions using omp get the threads count from GetRecommendedOMPThreadCount, so if omp is disabled they will get 1 thread and run serially
Copy link
Member

@szha szha left a comment

Choose a reason for hiding this comment

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

Have you tried such a build in jupyter notebooks? The python kernels are forked.

@larroy
Copy link
Contributor Author

larroy commented Dec 9, 2019

@szha no. What behaviour do you suggest would be reasonable under a notebook? Leave omp threads set to 1 or to the generous value. Is there a way to detect the runtime is Jupyter?

@leezu
Copy link
Contributor

leezu commented Dec 10, 2019

@szha do you have any reference on this? If you start python in your shell, the process is also forked. But as exec is executed after the fork, there are no problems.

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM

@anirudh2290 anirudh2290 merged commit 04ebe45 into apache:master Dec 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants