You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/06/24 17:40:17 UTC
[GitHub] [incubator-tvm] kevinthesun opened a new pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
kevinthesun opened a new pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918
TVM randomly binds master thread onto available cores. If there are multiple CPU sockets, it is possible that thread binds can cross sockets which causes NUMA. This PR limits the ranged of cores in TVM_NUM_THREADS so that on multi-socket machines we can limit thread binding with in a single socket.
@yidawang @FrozenGene
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918#discussion_r445093922
##########
File path: src/runtime/threading_backend.cc
##########
@@ -159,14 +159,8 @@ class ThreadGroup::Impl {
CPU_SET(sorted_order_[sorted_order_.size() - i - 1], &cpuset);
}
} else {
- int big_count = big_count_;
- // Imagine our x86 has cores 0 - 7
- // physical cores are 0 - 3, logical cores are 4 - 7, big_count_ is 8
- // we wish we run on physical cores, not logical cores to avoid contention issue.
Review comment:
The big count might be necessary for the big.little ARM targets
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918#discussion_r445119821
##########
File path: src/runtime/threading_backend.cc
##########
@@ -159,14 +159,8 @@ class ThreadGroup::Impl {
CPU_SET(sorted_order_[sorted_order_.size() - i - 1], &cpuset);
}
} else {
- int big_count = big_count_;
- // Imagine our x86 has cores 0 - 7
- // physical cores are 0 - 3, logical cores are 4 - 7, big_count_ is 8
- // we wish we run on physical cores, not logical cores to avoid contention issue.
Review comment:
Is this possible to detect big.LITTLE somewhere?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918#discussion_r445178372
##########
File path: src/runtime/threading_backend.cc
##########
@@ -159,14 +159,8 @@ class ThreadGroup::Impl {
CPU_SET(sorted_order_[sorted_order_.size() - i - 1], &cpuset);
}
} else {
- int big_count = big_count_;
- // Imagine our x86 has cores 0 - 7
- // physical cores are 0 - 3, logical cores are 4 - 7, big_count_ is 8
- // we wish we run on physical cores, not logical cores to avoid contention issue.
Review comment:
We need to look into the code. Perhaps do num_binds = std::min(MaxConcurrency(), big_count_); to keep backward compact for BIG.LITTLE
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918#discussion_r445178372
##########
File path: src/runtime/threading_backend.cc
##########
@@ -159,14 +159,8 @@ class ThreadGroup::Impl {
CPU_SET(sorted_order_[sorted_order_.size() - i - 1], &cpuset);
}
} else {
- int big_count = big_count_;
- // Imagine our x86 has cores 0 - 7
- // physical cores are 0 - 3, logical cores are 4 - 7, big_count_ is 8
- // we wish we run on physical cores, not logical cores to avoid contention issue.
Review comment:
OK, after taking a closer look at the code it may not be the problem
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #5918: [Thread Backend]Fix CPU Thread Binding for Multiple Sockets
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5918:
URL: https://github.com/apache/incubator-tvm/pull/5918#issuecomment-648995271
cc @FrozenGene @junrushao1994
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org