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

Manually track num_max_thread #12380

Merged
merged 8 commits into from
Nov 15, 2018
Merged

Manually track num_max_thread #12380

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e9ed61c
use cached version of get thread max
azai91 Aug 28, 2018
2c0b0e9
reserve core affects omp singleton
azai91 Aug 28, 2018
f9d8746
omp_thread_max_ updated in one line
azai91 Oct 19, 2018
9be6f2e
remove enabled block
azai91 Oct 19, 2018
e217013
add brackets
azai91 Oct 22, 2018
fbeb081
re-add excluded reserved
azai91 Nov 13, 2018
2145a7e
add missing var
azai91 Nov 13, 2018
467d812
refactor macro
azai91 Nov 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/engine/openmp.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,14 @@ void OpenMP::set_reserve_cores(int cores) {
CHECK_GE(cores, 0);
reserve_cores_ = cores;
#ifdef _OPENMP
if (reserve_cores_ >= omp_thread_max_) {
omp_set_num_threads(1);
} else {
omp_set_num_threads(omp_thread_max_ - reserve_cores_);
}
omp_thread_max_ = std::max(omp_thread_max_ - reserve_cores_, 1);
#endif
}

int OpenMP::GetRecommendedOMPThreadCount(bool exclude_reserved) const {
#ifdef _OPENMP
if (omp_num_threads_set_in_environment_) {
return omp_get_max_threads();
return omp_thread_max_;
}
if (enabled_) {
int thread_count = omp_get_max_threads();
Copy link
Contributor

Choose a reason for hiding this comment

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

You mention you want to call omp_get_max_threads() once and cache the results. This is another place where the API is called (in addition to line 55). Is that intentional? will it not create the same issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are the lines I changed. I removed the call and replaced it with the cached result.

the only call is in the constructor (line 55).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry - I was referring to line 90.

Copy link
Contributor

Choose a reason for hiding this comment

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

@azai91 Can you address @lupesko comments? I also found this change suspicious. It seems once omp_thread_max_ is set to 1 in line 77, all the subsequent call to this function will return 1 if the env variable is set. Which means, this stops multithreading altogether in the operator. Could you provide a test case to verify this change actually works? Thanks

Expand All @@ -101,10 +97,8 @@ int OpenMP::GetRecommendedOMPThreadCount(bool exclude_reserved) const {
}
return omp_thread_max_;
}
return 1;
#else
return 1;
#endif
return 1;
}

OpenMP *__init_omp__ = OpenMP::Get();
Expand Down