Add option to record C++ backtraces in _record_memory_history #86145
Conversation
I used this to debug #86136 so it is useful. Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86145
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 0f92a15: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
github-actions
bot
requested review from
Chillee,
Krovatkin,
albanD,
anjali411,
antoniojkim,
miladm and
wconstab
…ory" I used this to debug #86136 so it is useful. The implementation is not so fast so it is not enabled by default. Signed-off-by: Edward Z. Yang <ezyangfb.com> [ghstack-poisoned]
zdevito
left a comment
zdevito
left a comment
There was a problem hiding this comment.
This is useful, but the cost of Python (2 us) vs C++ stack tracing (however long it takes to to string format and gather a stack trace) is not on the same order of magnitude. It is done on every allocation so it happens a lot. This can lead people to think that enabling this for long running jobs is more expensive than it is (Python is pretty cheap), or for someone to think this is cheap but severely slow down the job they are looking at.
Maybe either warn that C++ stack traces are slow, or change the api to enable them to be more clear _enable_expensive_memory_history. There is probably a cheaper approach that walks the C++ stack and saves the addresses but does not formatting that can enable fast gathering of C++ stack traces in the future.
…ory" I used this to debug #86136 so it is useful. The implementation is not so fast so it is not enabled by default. Signed-off-by: Edward Z. Yang <ezyangfb.com> [ghstack-poisoned]
|
I made it guarded behind a scary looking kwarg, and also added a docblock for the function! |
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge job. Check the current status here. |
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
|
The merge job was canceled. If you believe this is a mistake,then you can re trigger it through pytorch-bot. |
|
@pytorchbot merge -f "all ready" |
|
@pytorchbot successfully started a merge job. Check the current status here. |
|
Hey @ezyang. |
#86145) Summary: I used this to debug #86136 so it is useful. The implementation is not so fast so it is not enabled by default. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Pull Request resolved: #86145 Approved by: https://github.com/albanD, https://github.com/zdevito Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/adf5919720c02dcf8c1ff32c890dd1c4e54d6fe7 Reviewed By: seemethere Differential Revision: D40167039 Pulled By: seemethere fbshipit-source-id: eaff41815b4dfc280dae266b375950cc5a20064a
Stack from ghstack (oldest at bottom):
I used this to debug #86136 so it is useful. The implementation is not so fast so it is not enabled by default.
Signed-off-by: Edward Z. Yang ezyang@fb.com