[Thread Backend]Fix CPU Thread Binding for Multiple Sockets #5918
Conversation
There was a problem hiding this comment.
The change looks good to me to fix the NUMA issue. Conceptually, this is a better setting as you don't want the master thread to freely jump to the cores that you are not required. However, generally speaking, I think the current setting (beyond the scope of this PR) doesn't consider the case of multiple instances running simultaneously, which could be a potential issue.
| 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. |
There was a problem hiding this comment.
The big count might be necessary for the big.little ARM targets
There was a problem hiding this comment.
Is this possible to detect big.LITTLE somewhere?
There was a problem hiding this comment.
We need to look into the code. Perhaps do num_binds = std::min(MaxConcurrency(), big_count_); to keep backward compact for BIG.LITTLE
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 within single socket.
@yidawang @FrozenGene