Skip to content
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

[Important] MKLDNN version of the code must have all space of memory #11481

Conversation

mozga-intel
Copy link
Contributor

@mozga-intel mozga-intel commented Jun 14, 2018

Problem with the MKLDNN performance after this change : commit

Description and solution
The mkldnn performance of the newest Paddle is a worse than the code before the changes: nearly 50%, The mkldnn version of code need to have all space of the memory to work correctly. During the tests with htop the loss of power of the threads is significant and we can not keep the fluency between the threads.

The code shows the solution to this problem. Summary: this is the fall back to the previous version of the code + MKLDNN flag.

@luotao1
Copy link
Contributor

luotao1 commented Jun 14, 2018

@mozga-intel I couldn't open

@mozga-intel
Copy link
Contributor Author

@luotao1. I am sorry. The link is up-to-date

@tensor-tang
Copy link
Contributor

tensor-tang commented Jun 14, 2018

Thanks @mozga-intel.

But I am afraid you can not change like this, because initial_cpu_memory_in_mb should also be used when WITH_MKLDNN.

The performance drop is only caused by the changes of memory size. You can try to get the necessary memory size for MKLDNN, like 1G not 500M. Then change the default size with ifdef PADDLE_WITH_MKLDNN.

Because online service also need the memory as a target, so we should keep as small memory as possible, The original default flag would take about 3% of system memory, which would be too large sometimes, like 6G,

That is the reason we add this flag.

Copy link
Contributor

@tensor-tang tensor-tang left a comment

Choose a reason for hiding this comment

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

Please try to keep this flag available no matter with or without MKLDNN.

The recommend way: change the default size when use MKLDNN. At least, we can know MKLDNN need how much memory to get the best performance.

@tensor-tang
Copy link
Contributor

@mozga-intel
I can refine it later, please wait a moment.

@mozga-intel
Copy link
Contributor Author

@tensor-tang Thank you for your quickly answer.

I stay in touch with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants