[PyTorch] Stack-allocate boxed args for RecordFunction #76266

swolchok · 2022-04-22T23:53:31Z

Stack from ghstack (oldest at bottom):

-> [PyTorch] Stack-allocate boxed args for RecordFunction #76266

Saving a heap allocation in this path improves performance.

Differential Revision: D34090699

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

facebook-github-bot · 2022-04-22T23:53:34Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/76266
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours
↩️ [fb-only] Re-run with SSH instructions

❌ 2 New Failures

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

Expand to see more

2/2 failures introduced in this PR

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages

trunk / linux-bionic-cuda10.2-py3.9-gcc7 / test (default, 2, 2, linux.4xlarge.nvidia.gpu) (1/2)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2022-05-20T17:48:12.2313759Z RuntimeError: test_sparse_csr failed!

2022-05-20T17:48:11.5246675Z 
2022-05-20T17:48:11.5246809Z Generating XML reports...
2022-05-20T17:48:11.6841472Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCSRCUDA-20220520174739.xml
2022-05-20T17:48:11.6843227Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCSRSampler-20220520174739.xml
2022-05-20T17:48:11.7354585Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCompressedCUDA-20220520174739.xml
2022-05-20T17:48:12.2308554Z Traceback (most recent call last):
2022-05-20T17:48:12.2311054Z   File "/var/lib/jenkins/workspace/test/run_test.py", line 1074, in <module>
2022-05-20T17:48:12.2311819Z     main()
2022-05-20T17:48:12.2312281Z   File "/var/lib/jenkins/workspace/test/run_test.py", line 1052, in main
2022-05-20T17:48:12.2312995Z     raise RuntimeError(err_message)
2022-05-20T17:48:12.2313759Z RuntimeError: test_sparse_csr failed!
2022-05-20T17:48:12.7866072Z 
2022-05-20T17:48:12.7866712Z real	6m31.042s
2022-05-20T17:48:12.7867040Z user	9m50.392s
2022-05-20T17:48:12.7867309Z sys	1m17.978s
2022-05-20T17:48:12.7867742Z + cleanup
2022-05-20T17:48:12.7868112Z + retcode=1
2022-05-20T17:48:12.7868488Z + set +x
2022-05-20T17:48:12.7918914Z ##[error]Process completed with exit code 1.
2022-05-20T17:48:12.7967158Z ##[group]Run pytorch/pytorch/.github/actions/get-workflow-job-id@master
2022-05-20T17:48:12.7967514Z with:

pull / linux-bionic-rocm5.1-py3.7 / test (default, 1, 2, linux.rocm.gpu) (2/2)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2022-05-20T18:40:52.8636762Z RuntimeError: test_sparse_csr failed!

2022-05-20T18:40:49.3638896Z 
2022-05-20T18:40:49.3639226Z Generating XML reports...
2022-05-20T18:40:49.6296118Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCSRCUDA-20220520184015.xml
2022-05-20T18:40:49.6320853Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCSRSampler-20220520184015.xml
2022-05-20T18:40:49.6881829Z Generated XML report: test-reports/python-unittest/test_sparse_csr/TEST-TestSparseCompressedCUDA-20220520184015.xml
2022-05-20T18:40:52.8617632Z Traceback (most recent call last):
2022-05-20T18:40:52.8618978Z   File "test/run_test.py", line 1074, in <module>
2022-05-20T18:40:52.8628622Z     main()
2022-05-20T18:40:52.8629753Z   File "test/run_test.py", line 1052, in main
2022-05-20T18:40:52.8635423Z     raise RuntimeError(err_message)
2022-05-20T18:40:52.8636762Z RuntimeError: test_sparse_csr failed!
2022-05-20T18:40:54.8404957Z 
2022-05-20T18:40:54.8405731Z real	6m34.281s
2022-05-20T18:40:54.8406521Z user	22m34.657s
2022-05-20T18:40:54.8407229Z sys	3m6.947s
2022-05-20T18:40:54.8408388Z + cleanup
2022-05-20T18:40:54.8409083Z + retcode=1
2022-05-20T18:40:54.8409736Z + set +x
2022-05-20T18:40:54.8541425Z ##[error]Process completed with exit code 1.
2022-05-20T18:40:54.8629772Z ##[group]Run # copy test results back to the mounted workspace, needed sudo, resulting permissions were correct
2022-05-20T18:40:54.8630305Z �[36;1m# copy test results back to the mounted workspace, needed sudo, resulting permissions were correct�[0m

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) ghstack-source-id: 154626794 Pull Request resolved: #76266

swolchok · 2022-04-22T23:58:57Z

An interesting follow-up would be to make it possible to avoid heap-allocating a Stack for calls to boxed KernelFunctions made via make_boxed_from_unboxed_functor. I have no idea how common this is, though.

ezyang · 2022-04-23T17:08:05Z

aten/src/ATen/core/dispatch/Dispatcher.h

+template <typename T>
+C10_DISPATCHER_INLINE_UNLESS_MOBILE void box(IValueStorage* dest, T& arg, int& lastIdx) {
+  new (&dest[lastIdx]) IValue(arg);
+  lastIdx++;


This needs to be kept consistent with our other boxing logic, is that right?

I'd suggest a cross-ref with aten/src/ATen/core/boxing/impl/boxing.h (or perhaps moving it to this file entirely)

ezyang · 2022-04-23T17:09:53Z

aten/src/ATen/core/dispatch/Dispatcher.h

+using IValueStorage = std::aligned_storage_t<sizeof(IValue), alignof(IValue)>;
+
+template <typename T>
+C10_DISPATCHER_INLINE_UNLESS_MOBILE void box(IValueStorage* dest, T& arg, int& lastIdx) {


The name here is a bit ambiguous; what we're doing is placement new boxing; would be nice if the name had this so people don't try to use this invalidly.

it's in detail and takes a weird typedef; they'll have trouble using it without writing obvious "here be dragons" stuff like aligned_storage

ezyang

mechanically looks all fine, just some naming / file placement stuff

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 154745500 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 154777631 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 154795837 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 155249947 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 155492055 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 155856030 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 155873881 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 156231743 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 156690927 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 156845963 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. ghstack-source-id: 156914882 Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/)

swolchok · 2022-05-24T17:05:22Z

@pytorchbot merge

github-actions · 2022-05-24T17:07:43Z

Hey @swolchok.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Pull Request resolved: #76266 Saving a heap allocation in this path improves performance. Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/80c4919bec29a8234e7289c058b905352d12d18e Reviewed By: aaronenyeshi Differential Revision: D34090699 Pulled By: swolchok fbshipit-source-id: 4a6a6623237e89de58e7bc350f5703726a09515e

[PyTorch] Stack-allocate boxed args for RecordFunction

ae857ef

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

facebook-github-bot added the cla signed label Apr 22, 2022

This was referenced Apr 22, 2022

[PyTorch] Make RecordFunction store inputs as ArrayRef #72484

Closed

[PyTorch] IValue(const c10::Scalar&) improvements #72551

Closed

swolchok requested review from robieta, ezyang, bdhirsh and bhosmer April 22, 2022 23:54

ezyang reviewed Apr 23, 2022

View reviewed changes

ezyang approved these changes Apr 23, 2022

View reviewed changes

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

1ad837d

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

af4c8e2

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

c53dab6

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

ccbd910

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

86ab13b

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

swolchok added the ciflow/all label May 9, 2022

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

71fde16

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

49ea9b8

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

31912af

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

9328cd7

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

6421ea5

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

Update on "[PyTorch] Stack-allocate boxed args for RecordFunction"

48895d1

Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]

pytorchmergebot added the Merged label May 24, 2022

pytorchmergebot closed this in 80c4919 May 24, 2022

facebook-github-bot deleted the gh/swolchok/501/head branch May 28, 2022 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PyTorch] Stack-allocate boxed args for RecordFunction #76266

[PyTorch] Stack-allocate boxed args for RecordFunction #76266

Uh oh!

swolchok commented Apr 22, 2022 •

edited

Loading

Uh oh!

facebook-github-bot commented Apr 22, 2022 •

edited

Loading

🕵️ 2 new failures recognized by patterns

trunk / linux-bionic-cuda10.2-py3.9-gcc7 / test (default, 2, 2, linux.4xlarge.nvidia.gpu) (1/2)

pull / linux-bionic-rocm5.1-py3.7 / test (default, 1, 2, linux.rocm.gpu) (2/2)

Uh oh!

swolchok commented Apr 22, 2022

Uh oh!

ezyang Apr 23, 2022

Uh oh!

ezyang Apr 23, 2022

Uh oh!

ezyang Apr 23, 2022

Uh oh!

swolchok Apr 25, 2022

Uh oh!

ezyang left a comment

Uh oh!

swolchok commented May 24, 2022

Uh oh!

github-actions bot commented May 24, 2022

Uh oh!

Uh oh!

[PyTorch] Stack-allocate boxed args for RecordFunction #76266

[PyTorch] Stack-allocate boxed args for RecordFunction #76266

Uh oh!

Conversation

swolchok commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

❌ 2 New Failures

🕵️ 2 new failures recognized by patterns

trunk / linux-bionic-cuda10.2-py3.9-gcc7 / test (default, 2, 2, linux.4xlarge.nvidia.gpu) (1/2)

pull / linux-bionic-rocm5.1-py3.7 / test (default, 1, 2, linux.rocm.gpu) (2/2)

Uh oh!

swolchok commented Apr 22, 2022

Uh oh!

ezyang Apr 23, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang Apr 23, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang Apr 23, 2022

Choose a reason for hiding this comment

Uh oh!

swolchok Apr 25, 2022

Choose a reason for hiding this comment

Uh oh!

ezyang left a comment

Choose a reason for hiding this comment

Uh oh!

swolchok commented May 24, 2022

Uh oh!

github-actions bot commented May 24, 2022

Uh oh!

Uh oh!

swolchok commented Apr 22, 2022 •

edited

Loading

facebook-github-bot commented Apr 22, 2022 •

edited

Loading