DTensor: avoid unnecessary DTensorSpec creation in _ToTorchTensor.backward#167588
DTensor: avoid unnecessary DTensorSpec creation in _ToTorchTensor.backward#167588swolchok wants to merge 3 commits intogh/swolchok/868/basefrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/167588
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 035049e with merge base 573a79f ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
[ghstack-poisoned]
ezyang
left a comment
There was a problem hiding this comment.
Huh, I'm surprised this actual is an improvement lol
| ): | ||
| # Avoid actual sharing of specs in case they're modified during (e.g.) | ||
| # sharding propagation. | ||
| grad_spec = copy.copy(dtensor_spec) |
There was a problem hiding this comment.
Is copying really that much faster?
copy in this case I would expect is actually doing a pickle serialization/deserialization of the object to get a new copy.
Is that really faster than creating a clean object below?
There was a problem hiding this comment.
Is that really faster than creating a clean object below?
Yes. DTensorSpec does some relatively expensive computation in its constructor
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…kward (pytorch#167588) Looks like the check here is cheap and has a potentially large payoff. Pull Request resolved: pytorch#167588 Approved by: https://github.com/ezyang
…kward (pytorch#167588) Looks like the check here is cheap and has a potentially large payoff. Pull Request resolved: pytorch#167588 Approved by: https://github.com/ezyang
``` git revert --no-commit 567dcdb 200156e 3d801a4 2034ca9 480b4ff f570e58 ``` And Revert "[DTensor] Document fast-path dispatch (#168192)" And Revert "[DTensor] Fix deadlock after fast cache clear (#168069)" Reverts: * #167860 * #167588 * #167475 * #166808 * #166372 * #168192 * #168069 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #168264 Approved by: https://github.com/seemethere, https://github.com/malfet
``` git revert --no-commit 567dcdb 200156e 3d801a4 2034ca9 480b4ff f570e58 ``` And Revert "[DTensor] Document fast-path dispatch (#168192)" And Revert "[DTensor] Fix deadlock after fast cache clear (#168069)" Reverts: * #167860 * #167588 * #167475 * #166808 * #166372 * #168192 * #168069 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #168264 Approved by: https://github.com/seemethere, https://github.com/malfet
Stack from ghstack (oldest at bottom):
Looks like the check here is cheap and has a potentially large payoff.
cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @pragupta @msaroufim @dcci