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
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 51 additions & 23 deletions sentry_sdk/profiler.py
Expand Up @@ -16,33 +16,31 @@
import platform
import random
import signal
import sys
import threading
import time
import sys
import uuid

from collections import deque
from collections import deque, namedtuple
from contextlib import contextmanager

import sentry_sdk
from sentry_sdk._compat import PY33

from sentry_sdk._types import MYPY
from sentry_sdk.utils import nanosecond_time

if MYPY:
from types import FrameType
from typing import Any
from typing import Deque
from typing import Dict
from typing import Generator
from typing import List
from typing import Optional
from typing import Sequence
from typing import Tuple
import sentry_sdk.tracing

Frame = Any
FrameData = Tuple[str, str, int]

FrameData = namedtuple("FrameData", ["name", "file", "line"])


_sample_buffer = None # type: Optional[_SampleBuffer]
Expand Down Expand Up @@ -127,7 +125,7 @@ def _sample_stack(*args, **kwargs):


def _extract_stack(frame):
# type: (Frame) -> Sequence[FrameData]
# type: (Optional[FrameType]) -> Sequence[FrameData]
"""
Extracts the stack starting the specified frame. The extracted stack
assumes the specified frame is the top of the stack, and works back
Expand All @@ -137,22 +135,52 @@ def _extract_stack(frame):
only the first `MAX_STACK_DEPTH` frames will be returned.
"""

stack = deque(maxlen=MAX_STACK_DEPTH) # type: Deque[FrameData]
stack = deque(maxlen=MAX_STACK_DEPTH) # type: Deque[FrameType]

while frame is not None:
stack.append(
(
# co_name only contains the frame name.
# If the frame was a class method,
# the class name will NOT be included.
frame.f_code.co_name,
frame.f_code.co_filename,
frame.f_code.co_firstlineno,
)
)
stack.append(frame)
frame = frame.f_back

return stack
return [
FrameData(
name=get_frame_name(frame),
file=frame.f_code.co_filename,
line=frame.f_lineno,
)
for frame in stack
]
Comment on lines +144 to +151
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.



def get_frame_name(frame):
# type: (FrameType) -> str

# in 3.11+, there is a frame.f_code.co_qualname that
# we should consider using instead where possible

# co_name only contains the frame name. If the frame was a method,
# the class name will NOT be included.
name = frame.f_code.co_name

# if it was a method, we can get the class name by inspecting
# the f_locals for the `self` argument
try:
if "self" in frame.f_locals:
return "{}.{}".format(frame.f_locals["self"].__class__.__name__, name)
except AttributeError:
pass

# if it was a class method, (decorated with `@classmethod`)
# we can get the class name by inspecting the f_locals for the `cls` argument
try:
if "cls" in frame.f_locals:
return "{}.{}".format(frame.f_locals["cls"].__name__, name)
except AttributeError:
pass

# nothing we can do if it is a staticmethod (decorated with @staticmethod)

# we've done all we can, time to give up and return what we have
return name


class Profile(object):
Expand Down Expand Up @@ -289,9 +317,9 @@ def slice_profile(self, start_ns, stop_ns):
frames[frame] = len(frames)
frames_list.append(
{
"name": frame[0],
"file": frame[1],
"line": frame[2],
"name": frame.name,
"file": frame.file,
"line": frame.line,
Comment on lines +318 to +320
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 👍

}
)
current_stack.append(frames[frame])
Expand Down