-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add mechanism for blocking oneDNN cache clearing #26502
Add mechanism for blocking oneDNN cache clearing #26502
Conversation
@luotao1 I've prepared this PR as a preview to my solution for conditionally clearing oneDNN cache. It's applied for now just in |
Thanks for your contribution! |
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.
It's applied for now just in run_program_op but can be extended to others like conditional_block_op and so on
I think it's OK, @zhhsplendid @chenwhql Could you help review it?
@@ -262,6 +265,9 @@ class RunProgramOpKernel : public framework::OpKernel<T> { | |||
} | |||
VLOG(2) << "The number of sub scopes after forward: " | |||
<< out_scope_vec->front()->kids().size(); | |||
#ifdef PADDLE_WITH_MKLDNN | |||
DontClearMKLDNNCache(ctx.GetPlace()); | |||
#endif |
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.
#ifdef PADDLE_WITH_MKLDNN
if(FLAGS_use_mkldnn)
DontClearMKLDNNCache(ctx.GetPlace());
#endif
@@ -506,6 +509,7 @@ class MKLDNNDeviceContext : public CPUDeviceContext { | |||
mkldnn::engine engine_; | |||
std::shared_ptr<BlobMap> p_blobmap_; | |||
std::shared_ptr<std::mutex> p_mutex_; | |||
bool block_next_cache_clearing = false; |
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.
Our private members are named with a trailing underscore, like line 509 - line 512.
Could you rename it to "block_next_cache_clearing_" ?
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
PR types
New features
PR changes
Others
Describe
This PR adds a mechanism for preventing oneDNN cache clearing without changes in Executor. It solves problems mentioned in #25988.