Skip to content
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

Merged
merged 8 commits into from Oct 13, 2022

Conversation

Zylphrex
Copy link
Member

@Zylphrex Zylphrex commented Oct 7, 2022

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

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
@Zylphrex Zylphrex requested review from sl0thentr0py and a team October 7, 2022 17:31
Comment on lines +144 to +151
return [
FrameData(
name=get_frame_name(frame),
file=frame.f_code.co_filename,
line=frame.f_lineno,
)
for frame in stack
]
Copy link
Member Author

@Zylphrex Zylphrex Oct 7, 2022

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

Figure_1

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.

Comment on lines +320 to +322
"name": frame.name,
"file": frame.file,
"line": frame.line,
Copy link
Contributor

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.

Copy link
Member Author

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 👍

@Zylphrex Zylphrex merged commit ed0d4db into master Oct 13, 2022
@Zylphrex Zylphrex deleted the txiao/feat/extract-qualified-name-for-each-frame branch October 13, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants