[PyTorch] Stack-allocate boxed args for RecordFunction#76266
[PyTorch] Stack-allocate boxed args for RecordFunction#76266swolchok wants to merge 12 commits intogh/swolchok/501/basefrom
Conversation
Saving a heap allocation in this path improves performance. Differential Revision: [D34090699](https://our.internmc.facebook.com/intern/diff/D34090699/) [ghstack-poisoned]
🔗 Helpful links
❌ 2 New FailuresAs of commit 48895d1 (more details on the Dr. CI page): Expand to see more
🕵️ 2 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
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
|
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. |
| template <typename T> | ||
| C10_DISPATCHER_INLINE_UNLESS_MOBILE void box(IValueStorage* dest, T& arg, int& lastIdx) { | ||
| new (&dest[lastIdx]) IValue(arg); | ||
| lastIdx++; |
There was a problem hiding this comment.
This needs to be kept consistent with our other boxing logic, is that right?
There was a problem hiding this comment.
I'd suggest a cross-ref with aten/src/ATen/core/boxing/impl/boxing.h (or perhaps moving it to this file entirely)
| 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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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/)
|
@pytorchbot merge |
|
Hey @swolchok. |
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
Stack from ghstack (oldest at bottom):
Saving a heap allocation in this path improves performance.
Differential Revision: D34090699