Support record_shapes in RPC profiling #44419

rohan-varma · 2020-09-09T20:21:55Z

Stack from ghstack:

Support record_shapes in RPC profiling #44419 Support record_shapes in RPC profiling

This PR adds support for propagation of input shapes over the wire when the profiler is invoked with record_shapes=True over RPC. Previously, we did not respect this argument.

This is done by saving the shapes as an ivalue list and recovering it as the type expected (std::vector<std::vector<int>> on the client). Test is added to ensure that remote ops have the same input_shapes as if the op were run locally.

Remote ops profiler output before (run with group_by_input_shapes=True):

rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add        0.18%            67.229us         0.21%            76.449us         76.449us         1                2                []
rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty      0.03%            9.220us          0.03%            9.220us          9.220us          1                2                []

After:

rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add        0.14%            91.001us         0.16%            102.094us        102.094us        1                2                [[100], [100], []]
rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty      0.02%            11.093us         0.02%            11.093us         11.093us         1                2                [[], [], [], [], [], []]

Differential Revision: D23591274

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

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! ghstack-source-id: 111702390 Pull Request resolved: #44419

dr-ci · 2020-09-09T23:19:17Z

💊 CI failures summary and remediations

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

✅ None of the CI failures appear to be your fault 💚

2/2 broken upstream at merge base d9af3d2 since Sep 25

🚧 2 ongoing upstream failures:

These were probably caused by upstream breakages that are not fixed yet:

pytorch_python_doc_build since Sep 25
- 🔁 rerun
pytorch_doc_test since Sep 25
- 🔁 rerun

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 40 times.

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 111932779 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 111996895 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 112200516 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 112320646 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

codecov · 2020-09-18T00:52:07Z

Codecov Report

❗ No coverage uploaded for pull request base (gh/rohan-varma/166/base@0c75e4b). Click here to learn what that means.
The diff coverage is n/a.

@@                    Coverage Diff                     @@
##             gh/rohan-varma/166/base   #44419   +/-   ##
==========================================================
  Coverage                           ?   68.04%           
==========================================================
  Files                              ?      393           
  Lines                              ?    50937           
  Branches                           ?        0           
==========================================================
  Hits                               ?    34659           
  Misses                             ?    16278           
  Partials                           ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c75e4b...8772f81. Read the comment docs.

pritamdamania87 · 2020-09-18T01:19:12Z

Could you paste the before and after autograd profiler results to the PR summary so its easy to see the appropriate changes in the profiler output?

pritamdamania87

Unit test looks good to me, will let @ilia-cher sign off on the profiler.cpp changes.

rohan-varma · 2020-09-18T17:45:04Z

Could you paste the before and after autograd profiler results to the PR summary so its easy to see the appropriate changes in the profiler output?

@pritamdamania87 The before/after of the RPC remote events are in the summary now.

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Remote ops profiler output before (run with `group_by_input_shapes=True`): ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.18% 67.229us 0.21% 76.449us 76.449us 1 2 [] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.03% 9.220us 0.03% 9.220us 9.220us 1 2 [] ``` After: ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.14% 91.001us 0.16% 102.094us 102.094us 1 2 [[100], [100], []] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.02% 11.093us 0.02% 11.093us 11.093us 1 2 [[], [], [], [], [], []] ``` Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 112390206 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

ilia-cher · 2020-09-24T14:43:33Z

torch/csrc/autograd/profiler.cpp

+  shapesFromServer.reserve(shapeList.size());
+  for (size_t i = 0 ; i < shapeList.size(); ++i) {
+    std::vector<int64_t> s;
+    auto curShapesList = shapeList.get(i).toList();


should we also check isList here?

ilia-cher · 2020-09-24T14:43:59Z

torch/csrc/autograd/profiler.cpp

      " elements to reconstruct Event.");

+  // Reconstruct input shapes from ivalues.
+  auto shapeListFromServer = ivalues.get(EventIValueIdx::SHAPES);


nit: could you remove mentions of server?

Removed all of the mentions.

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Remote ops profiler output before (run with `group_by_input_shapes=True`): ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.18% 67.229us 0.21% 76.449us 76.449us 1 2 [] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.03% 9.220us 0.03% 9.220us 9.220us 1 2 [] ``` After: ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.14% 91.001us 0.16% 102.094us 102.094us 1 2 [[100], [100], []] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.02% 11.093us 0.02% 11.093us 11.093us 1 2 [[], [], [], [], [], []] ``` Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 112822760 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

ilia-cher

LG!

Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. Remote ops profiler output before (run with `group_by_input_shapes=True`): ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.18% 67.229us 0.21% 76.449us 76.449us 1 2 [] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.03% 9.220us 0.03% 9.220us 9.220us 1 2 [] ``` After: ``` rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::add 0.14% 91.001us 0.16% 102.094us 102.094us 1 2 [[100], [100], []] rpc_sync#aten::add(worker1 -> worker2)#remote_op: aten::empty 0.02% 11.093us 0.02% 11.093us 11.093us 1 2 [[], [], [], [], [], []] ``` Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)! [ghstack-poisoned]

Pull Request resolved: #44419 Closes #39969 This PR adds support for propagation of input shapes over the wire when the profiler is invoked with `record_shapes=True` over RPC. Previously, we did not respect this argument. This is done by saving the shapes as an ivalue list and recovering it as the type expected (`std::vector<std::vector<int>>` on the client). Test is added to ensure that remote ops have the same `input_shapes` as if the op were run locally. ghstack-source-id: 112977899 Differential Revision: [D23591274](https://our.internmc.facebook.com/intern/diff/D23591274/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23591274/)!

facebook-github-bot · 2020-09-26T22:11:29Z

This pull request has been merged in 23dfca8.

rohan-varma requested review from albanD, apaszke, mrshenli, pritamdamania87 and zhaojuanmao as code owners September 9, 2020 20:21

rohan-varma mentioned this pull request Sep 9, 2020

TorchScript with record_function #44345

Closed

rohan-varma requested a review from ilia-cher September 9, 2020 20:23

pritamdamania87 reviewed Sep 18, 2020

View reviewed changes

ilia-cher reviewed Sep 24, 2020

View reviewed changes

rohan-varma requested a review from ilia-cher September 24, 2020 17:37

ilia-cher approved these changes Sep 25, 2020

View reviewed changes

facebook-github-bot closed this in 23dfca8 Sep 26, 2020

facebook-github-bot added the merged label Sep 26, 2020

facebook-github-bot deleted the gh/rohan-varma/166/head branch September 30, 2020 14:17

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support record_shapes in RPC profiling #44419

Support record_shapes in RPC profiling #44419

Uh oh!

rohan-varma commented Sep 9, 2020 •

edited

Loading

Uh oh!

dr-ci bot commented Sep 9, 2020 •

edited

Loading

Uh oh!

codecov bot commented Sep 18, 2020 •

edited

Loading

Uh oh!

pritamdamania87 commented Sep 18, 2020

Uh oh!

pritamdamania87 left a comment

Uh oh!

rohan-varma commented Sep 18, 2020

Uh oh!

ilia-cher Sep 24, 2020

Uh oh!

ilia-cher Sep 24, 2020

Uh oh!

rohan-varma Sep 24, 2020

Uh oh!

ilia-cher left a comment

Uh oh!

facebook-github-bot commented Sep 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Support record_shapes in RPC profiling #44419

Support record_shapes in RPC profiling #44419

Uh oh!

Conversation

rohan-varma commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🚧 2 ongoing upstream failures:

Uh oh!

codecov bot commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pritamdamania87 commented Sep 18, 2020

Uh oh!

pritamdamania87 left a comment

Choose a reason for hiding this comment

Uh oh!

rohan-varma commented Sep 18, 2020

Uh oh!

ilia-cher Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

ilia-cher Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

rohan-varma Sep 24, 2020

Choose a reason for hiding this comment

Uh oh!

ilia-cher left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rohan-varma commented Sep 9, 2020 •

edited

Loading

dr-ci bot commented Sep 9, 2020 •

edited

Loading

codecov bot commented Sep 18, 2020 •

edited

Loading