Skip to content

[r2.5] Update of oneDNN version for mkl_aarch64 build target#48543

Merged
mihaimaruseac merged 2 commits intotensorflow:r2.5from
alenik01:mkl_aarch64_update
Apr 22, 2021
Merged

[r2.5] Update of oneDNN version for mkl_aarch64 build target#48543
mihaimaruseac merged 2 commits intotensorflow:r2.5from
alenik01:mkl_aarch64_update

Conversation

@alenik01
Copy link
Contributor

This PR noticeably improves the performance of CNNs on AArch64 (--config=mkl_aarch64). oneDNN version upgrade follows the PR #48035

@google-cla google-cla bot added the cla: yes label Apr 15, 2021
@alenik01
Copy link
Contributor Author

@penpornk @agramesh1 please have a look.

@penpornk
Copy link
Member

penpornk commented Apr 15, 2021

@alenik01 Oh, I thought you are going to upgrade this in master and not 2.5.

For 2.5, I'm not sure if we can cherry-pick this, since cherry-picks are for security / bug fixes. Upgrading from a v2.1 release to v2.2 release might count more towards adding features rather than fixes. (The upgrade we did here was from a random commit after v2.2-rc to v2.2 for more stability and fixes.)

@mihaimaruseac What do you think?

Co-authored-by: Penporn Koanantakool <38085909+penpornk@users.noreply.github.com>
@alenik01
Copy link
Contributor Author

alenik01 commented Apr 15, 2021

@penpornk if it is possible to cherry-pick this for v2.5 release, then it would be nice (the problem was that oneDNN v2.2 had been released after TF feature freeze deadline). If not, then this PR can be merged in the master.

@penpornk
Copy link
Member

@alenik01 Could you please share how much performance improvements you observe from this update? Is this update for performance improvements alone or is it also fixing some issues?

@penpornk
Copy link
Member

@alenik01 Also, could you please create another PR for the master branch? We usually only merge a cherry-pick after seeing the same changes successfully merged (and passed nightly tests) in master.

I will check with the team if this PR is eligible for cherry-picking when I hear back about the speedups and whether this fixes any issues as well. Thank you!

@alenik01
Copy link
Contributor Author

@penpornk

Could you please share how much performance improvements you observe from this update?

The speedup is noticeable, x1.7 (was measure for ResNet50 inference).

Is this update for performance improvements alone or is it also fixing some issues?

Performance alone.

Also, could you please create another PR for the master branch?

OK, I will create one.

since cherry-picks are for security / bug fixes

Many thanks for clarifying, now I understand that the version update for mkl_dnn_v1 was one-off case.

@nSircombe
Copy link
Contributor

@mihaimaruseac - this is the PR we briefly discussed at the SIG build meeting the other week.

@alenik01
Copy link
Contributor Author

@penpornk

Also, could you please create another PR for the master branch?

Here it is: #48574

@gbaned gbaned self-assigned this Apr 19, 2021
@gbaned gbaned added the size:S CL Change Size: Small label Apr 19, 2021
@gbaned gbaned assigned goldiegadde and unassigned gbaned Apr 19, 2021
@geetachavan1 geetachavan1 added the kokoro:force-run Tests on submitted change label Apr 19, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 19, 2021
@mihaimaruseac mihaimaruseac merged commit bfda315 into tensorflow:r2.5 Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes size:S CL Change Size: Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants