8000 ref(profiling): Eagerly hash stack for profiles (#1755) · getsentry/sentry-python@01dc7ee · GitHub
[go: up one dir, main page]

Skip to content

Commit 01dc7ee

Browse files
authored
ref(profiling): Eagerly hash stack for profiles (#1755)
Hashing the stack is an expensive operation and the same stack is used for parallel transactions happening on various threads. Instead of hashing it each time it's used.
1 parent 905b3fd commit 01dc7ee

File tree

2 files changed

+37
-32
lines changed

2 files changed

+37
-32
lines changed

sentry_sdk/profiler.py

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
from typing_extensions import TypedDict
5454
import sentry_sdk.tracing
5555

56-
RawSampleData = Tuple[int, Sequence[Tuple[str, Sequence[RawFrameData]]]]
56+
RawStack = Tuple[RawFrameData, ...]
57+
RawSample = Sequence[Tuple[str, RawStack]]
58+
RawSampleWithId = Sequence[Tuple[str, int, RawStack]]
5759

5860
ProcessedStack = Tuple[int, ...]
5961

@@ -153,7 +155,7 @@ def teardown_profiler():
153155

154156

155157
def extract_stack(frame, max_stack_depth=MAX_STACK_DEPTH):
156-
# type: (Optional[FrameType], int) -> Sequence[RawFrameData]
158+
# type: (Optional[FrameType], int) -> Tuple[RawFrameData, ...]
157159
"""
158160
Extracts the stack starting the specified frame. The extracted stack
159161
assumes the specified frame is the top of the stack, and works back
@@ -328,12 +330,14 @@ class SampleBuffer(object):
328330
def __init__(self, capacity):
329331
# type: (int) -> None
330332

331-
self.buffer = [None] * capacity # type: List[Optional[RawSampleData]]
333+
self.buffer = [
334+
None
335+
] * capacity # type: List[Optional[Tuple[int, RawSampleWithId]]]
332336
self.capacity = capacity # type: int
333337
self.idx = 0 # type: int
334338

335-
def write(self, sample):
336-
# type: (RawSampleData) -> None
339+
def write(self, ts, raw_sample):
340+
# type: (int, RawSample) -> None
337341
"""
338342
Writing to the buffer is not thread safe. There is the possibility
339343
that parallel writes will overwrite one another.
@@ -346,7 +350,24 @@ def write(self, sample):
346350
any synchronization mechanisms here like locks.
347351
"""
348352
idx = self.idx
349-
self.buffer[idx] = sample
353+
354+
sample = [
355+
(
356+
thread_id,
357+
# Instead of mapping the stack into frame ids and hashing
358+
# that as a tuple, we can directly hash the stack.
359+
# This saves us from having to generate yet another list.
360+
# Additionally, using the stack as the key directly is
361+
# costly because the stack can be large, so we pre-hash
362+
# the stack, and use the hash as the key as this will be
363+
# needed a few times to improve performance.
364+
hash(stack),
365+
stack,
366+
)
367+
for thread_id, stack in raw_sample
368+
]
369+
370+
self.buffer[idx] = (ts, sample)
350371
self.idx = (idx + 1) % self.capacity
351372

352373
def slice_profile(self, start_ns, stop_ns):
@@ -357,27 +378,13 @@ def slice_profile(self, start_ns, stop_ns):
357378
frames = dict() # type: Dict[RawFrameData, int]
358379
frames_list = list() # type: List[ProcessedFrame]
359380

360-
# TODO: This is doing an naive iteration over the
361-
# buffer and extracting the appropriate samples.
362-
#
363-
# Is it safe to assume that the samples are always in
364-
# chronological order and binary search the buffer?
365381
for ts, sample in filter(None, self.buffer):
366382
if start_ns > ts or ts > stop_ns:
367383
continue
368384

369385
elapsed_since_start_ns = str(ts - start_ns)
370386

371-
for tid, stack in sample:
372-
# Instead of mapping the stack into frame ids and hashing
373-
# that as a tuple, we can directly hash the stack.
374-
# This saves us from having to generate yet another list.
375-
# Additionally, using the stack as the key directly is
376-
# costly because the stack can be large, so we pre-hash
377-
# the stack, and use the hash as the key as this will be
378-
# needed a few times to improve performance.
379-
hashed_stack = hash(stack)
380-
387+
for tid, hashed_stack, stack in sample:
381388
# Check if the stack is indexed first, this lets us skip
382389
# indexing frames if it's not necessary
383390
if hashed_stack not in stacks:
@@ -433,13 +440,11 @@ def _sample_stack(*args, **kwargs):
433440
"""
434441

435442
self.write(
436-
(
437-
nanosecond_time(),
438-
[
439-
(str(tid), extract_stack(frame))
440-
for tid, frame in sys._current_frames().items()
441-
],
442-
)
443+
nanosecond_time(),
444+
[
445+
(str(tid), extract_stack(frame))
446+
for tid, frame in sys._current_frames().items()
447+
],
443448
)
444449

445450
return _sample_stack

tests/test_profiler.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,8 @@ def __init__(self, capacity, sample_data=None):
249249

250250
def make_sampler(self):
251251
def _sample_stack(*args, **kwargs):
252-
print("writing", self.sample_data[0])
253-
self.write(self.sample_data.pop(0))
252+
ts, sample = self.sample_data.pop(0)
253+
self.write(ts, sample)
254254

255255
return _sample_stack
256256

@@ -760,7 +760,7 @@ def test_thread_scheduler_single_background_thread(scheduler_class):
760760
)
761761
def test_sample_buffer(capacity, start_ns, stop_ns, samples, profile):
762762
buffer = SampleBuffer(capacity)
763-
for sample in samples:
764-
buffer.write(sample)
763+
for ts, sample in samples:
764+
buffer.write(ts, sample)
765765
result = buffer.slice_profile(start_ns, stop_ns)
766766
assert result == profile

0 commit comments

Comments
 (0)
0