Skip to content

Fix max_pool2d perf regression #41174

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.

Sign up for GitHub

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

Jump to bottom
Closed
xwang233 wants to merge 1 commit into from
Closed

Fix max_pool2d perf regression #41174

xwang233 wants to merge 1 commit into from

Conversation

@xwang233
Copy link
Collaborator

@xwang233 xwang233 commented Jul 9, 2020 •
edited
Loading

The two pointer variables ptr_top_diff and ptr_top_mask were introduced in #38953. Some end-to-end tests showed training performance regression due to this change. The performance is restored after removing the two pointer variables, and adding offset directly below in the indexing [ ] calculations.

See PR change https://github.com/pytorch/pytorch/pull/38953/files#diff-8085d370f4e98295074a51b8a1f829e9R187-R188

pytorch/aten/src/ATen/native/cuda/DilatedMaxPool2d.cu

Lines 186 to 195 in e4a3c58

int offset = (n * channels + c) * pooled_height * pooled_width;
const scalar_t* ptr_top_diff = top_diff + offset;
const int64_t* ptr_top_mask = top_mask + offset;
for (int ph = phstart; ph < phend; ++ph) {
for (int pw = pwstart; pw < pwend; ++pw) {
if (ptr_top_mask[ph * pooled_width + pw] == h * width + w) {
gradient += ScalarConvert<scalar_t, accscalar_t>::to(ptr_top_diff[ph * pooled_width + pw]);
}
}
}

@xwang233
Copy link
Collaborator Author

xwang233 commented Jul 9, 2020

cc @ptrblck @mcarilli

@xwang233 xwang233 requested a review from ngimel July 9, 2020 05:10
facebook-github-bot
facebook-github-bot reviewed Jul 9, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ngimel has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 9, 2020

@ngimel merged this pull request in 2cf31fb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@ngimel ngimel ngimel approved these changes

Assignees

No one assigned

Labels

Merged open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xwang233 @facebook-github-bot @ngimel @pytorchbot @mruberry