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

[PyTorch] Fix getCustomClassType() perf #48981

Closed
wants to merge 4 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Dec 8, 2020

Stack from ghstack:

  1. It was copying the entire hash table every time.
  2. We don't need to do a hash lookup at all.

Differential Revision: D25385543

NOTE FOR REVIEWERS: This PR has internal Facebook specific changes or comments, please review them on Phabricator!

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Dec 8, 2020
1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!

ghstack-source-id: 118048320
Pull Request resolved: #48981
@dr-ci
Copy link

dr-ci bot commented Dec 8, 2020

💊 CI failures summary and remediations

As of commit 8525492 (more details on the Dr. CI page):


  • 3/3 failures possibly* introduced in this PR
    • 3/3 non-CircleCI failure(s)

Extra GitHub checks: 2 failed


codecov.io: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 12 times.

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Dec 8, 2020
Pull Request resolved: #48981

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.
ghstack-source-id: 118065080

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!
@codecov
Copy link

codecov bot commented Dec 8, 2020

Codecov Report

Merging #48981 (8525492) into gh/swolchok/35/base (274ce26) will decrease coverage by 0.00%.
The diff coverage is 77.77%.

@@                   Coverage Diff                   @@
##           gh/swolchok/35/base   #48981      +/-   ##
=======================================================
- Coverage                80.74%   80.73%   -0.01%     
=======================================================
  Files                     1869     1869              
  Lines                   201654   201647       -7     
=======================================================
- Hits                    162818   162796      -22     
- Misses                   38836    38851      +15     

…perf"

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!

[ghstack-poisoned]
…] Fix getCustomClassType() perf"

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Dec 9, 2020
Pull Request resolved: #48981

1) It was copying the entire hash table every time.
2) We don't need to do a hash lookup at all.
ghstack-source-id: 118164406

Differential Revision: [D25385543](https://our.internmc.facebook.com/intern/diff/D25385543/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D25385543/)!
@facebook-github-bot facebook-github-bot deleted the gh/swolchok/35/head branch December 15, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants