-
Notifications
You must be signed in to change notification settings - Fork 515
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
feat(profiling): Extract qualified name for each frame #1669
feat(profiling): Extract qualified name for each frame #1669
Conversation
Currently, we use `code.co_name` for the frame name. This does not include the name of the class if it was a method. This tries to extract the qualified name for each frame where possible. - methods: *typically* have `self` as a positional argument and we can inspect it to extract the class name - class methods: *typically* have `cls` as a positional argument and we can inspect it to extract the class name - static methods: no obvious way of extract the class name
return [ | ||
FrameData( | ||
name=get_frame_name(frame), | ||
file=frame.f_code.co_filename, | ||
line=frame.f_lineno, | ||
) | ||
for frame in stack | ||
] |
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.
Instead of extracting the data for every frame while we step through the stack, we step through the stack first and only extract the data from the frames within the MAX_STACK_DEPTH
.
Some quick benchmarks on 100,000 iterations
- stack depth of 100: 4.22s vs 4.55s (a little slower)
- stack depth of 500: 21.76s vs 8.32 (a lot faster)
Illustration for convenience
Since the slowest part of this is the get_frame_name
function and I figured it was worth the trade off here to optimize for the worst case and be a little slower on the common cases.
"name": frame.name, | ||
"file": frame.file, | ||
"line": frame.line, |
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.
If you don't mind using function
/filename
/lineno
/colno
, that'd be great. We renamed them to align with the Frame
object in the protocol but still provide compatibility though.
Also, we added colno
in case you want/can report the column.
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 have this in a follow up 👍
Currently, we use
code.co_name
for the frame name. This does not include the name of the class if it was a method. This tries to extract the qualified name for each frame where possible.self
as a positional argument and we can inspect it to extract the class namecls
as a positional argument and we can inspect it to extract the class name