-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[CPU] Setting parallel to tbb static partitioner or tbb auto partitoner by thread pool #30167
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
base: master
Are you sure you want to change the base?
[CPU] Setting parallel to tbb static partitioner or tbb auto partitoner by thread pool #30167
Conversation
src/plugins/intel_cpu/src/nodes/executors/dnnl/dnnl_matmul_primitive.cpp
Outdated
Show resolved
Hide resolved
constexpr int default_multiplier = 32; | ||
class ThreadPool; | ||
|
||
class CpuParallel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT. Could you please move it inside the class as a static field?
class CpuParallel {
public:
static constexpr int default_multiplier = 32;
public:
CpuParallel() = delete;
....
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok moved it inside CpuParallel class.
template <typename T0, typename R, typename F> | ||
[[nodiscard]] R cpu_parallel_sum(const T0& D0, const R& input, const F& func) const { | ||
#if OV_THREAD == OV_THREAD_TBB_ADAPTIVE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move all the cpu_parallel_...
methods into the private section? Because with this design, the cpu_parallel_...
methods and the parallel_...
methods of the same class may be used interchangeably, which would introduce a mess into the code base. Better to avoid such a situation and use the uniform name convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right, moved these functions into private section.
// cpu stream executor for current graph | ||
ov::threading::CPUStreamsExecutor::Ptr m_cpuStreamExecutor; | ||
std::shared_ptr<CpuParallel> m_cpuParallel = nullptr; | ||
// std::shared_ptr<ThreadPool> m_threadPool = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Excellent work!
@sunxiaoxia2022 , could you please fix the CI errors? |
Details:
Tickets: