Skip to content

feat(profiling): Extract qualified name for each frame - #1669

Merged
Zylphrex merged 8 commits into
masterfrom
txiao/feat/extract-qualified-name-for-each-frame
Oct 13, 2022
Merged

Zylphrex merged 8 commits into
masterfrom
txiao/feat/extract-qualified-name-for-each-frame

Conversation

@Zylphrex

@Zylphrex Zylphrex commented Oct 7, 2022

Copy link
Copy Markdown
Member

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 a team and sl0thentr0py October 7, 2022 17:31
Comment thread sentry_sdk/profiler.py
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
]

@Zylphrex Zylphrex Oct 7, 2022 •

Copy link
Copy Markdown
Member Author

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 thread sentry_sdk/profiler.py
Comment on lines +320 to +322
"name": frame.name,
"file": frame.file,
"line": frame.line,

Copy link
Copy Markdown
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
Copy Markdown
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.

3 participants