-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove unnecessary stack frame suffix #24
Conversation
Stack from ghstack (oldest at bottom): |
There's a bug in our current frame emission code where we emit three Dynamo frames. Actually, I think this is fixed in PyTorch main but there's legacy code still running it. Chop it off. Signed-off-by: Edward Z. Yang <[email protected]> ghstack-source-id: ff288d7e1465442cd10efbcdb64ea8e339841738 ghstack-comment-id: 2087103294 Pull Request resolved: #24
&& frame.name == target.1 | ||
}) | ||
{ | ||
frames.truncate(len - 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: truncate to target_frames.len() here
@@ -23,6 +23,29 @@ pub struct ParseConfig { | |||
pub custom_parsers: Vec<Box<dyn crate::parsers::StructuredLogParser>>, | |||
} | |||
|
|||
fn maybe_remove_suffix(frames: &mut Vec<FrameSummary>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like you'd want to name this something more specific like maybe_remove_convert_frame_suffixes
, or make the function more general by being able to pass in a list of target frames instead of hard coding it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, #30
Signed-off-by: Edward Z. Yang <[email protected]> ghstack-source-id: 1ebd007fc1fe0b91200e0083d1238a5317292c6c ghstack-comment-id: 2088607801 Pull Request resolved: #30
Signed-off-by: Edward Z. Yang <[email protected]> ghstack-source-id: 200a5aa7b8d6cdd24d76cb2e168df15e309c0c1a ghstack-comment-id: 2088607801 Pull Request resolved: #30
There's a bug in our current frame emission code where we emit three
Dynamo frames. Actually, I think this is fixed in PyTorch main but
there's legacy code still running it. Chop it off.
Signed-off-by: Edward Z. Yang [email protected]