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] Make tls_local_dispatch_key_set inlineable (reapply) #49412

Closed
wants to merge 2 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Dec 15, 2020

Stack from ghstack:

FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.

Differential Revision: D25547962

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

FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.

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

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

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 15, 2020

💊 CI failures summary and remediations

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


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

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 to the (internal) Dr. CI Users group.

This comment has been revised 9 times.

swolchok added a commit that referenced this pull request Dec 15, 2020
FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.

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

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

ghstack-source-id: 118624599
Pull Request resolved: #49412
@swolchok swolchok requested a review from ezyang December 15, 2020 17:58
@ezyang
Copy link
Contributor

ezyang commented Dec 15, 2020

Yeah.... Scott, I'm pretty sure this error is why it wasn't inlined on Windows. Maybe that just means we should only inline on Linux though.

… inlineable (reapply)"

FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.

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

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

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

FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.
ghstack-source-id: 118669590

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

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

codecov bot commented Dec 16, 2020

Codecov Report

Merging #49412 (09a59b9) into gh/swolchok/48/base (8abbcf1) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@                   Coverage Diff                   @@
##           gh/swolchok/48/base   #49412      +/-   ##
=======================================================
- Coverage                80.57%   80.56%   -0.01%     
=======================================================
  Files                     1875     1875              
  Lines                   202706   202702       -4     
=======================================================
- Hits                    163321   163309      -12     
- Misses                   39385    39393       +8     

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

okey dokey

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 6f928a4.

ezyang added a commit that referenced this pull request Dec 17, 2020
ezyang added a commit that referenced this pull request Dec 17, 2020
#49412)"

This reverts commit 6f928a4.

ghstack-source-id: 11fc66261adfd2748aee88dfe75928aaa790f9ec
Pull Request resolved: #49548
@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by 19dc5e9.

@facebook-github-bot facebook-github-bot deleted the gh/swolchok/48/head branch December 20, 2020 15:18
hwangdeyu pushed a commit to hwangdeyu/pytorch that referenced this pull request Jan 6, 2021
…ch#49412)

Summary:
Pull Request resolved: pytorch#49412

FLAGS_disable_variable_dispatch had to go, but it looks like the only user was some benchmarks anyway.
ghstack-source-id: 118669590

Test Plan:
Small (order of 0.1% improvement) on Internal benchmarks. Wait for
GitHub CI since this was reverted before due to CI break

Reviewed By: ezyang

Differential Revision: D25547962

fbshipit-source-id: 58424b1da230fdc5d27349af762126a5512fce43
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

3 participants