Skip to content

feat(profiling): Introduce different profiler schedulers#1616

Merged
Zylphrex merged 7 commits into
masterfrom
txiao/feat/introduce-different-profiler-schedulers
Sep 19, 2022
Merged

feat(profiling): Introduce different profiler schedulers#1616
Zylphrex merged 7 commits into
masterfrom
txiao/feat/introduce-different-profiler-schedulers

Conversation

@Zylphrex

Copy link
Copy Markdown
Member

Previously, the only scheduling mechanism was via signals.SIGPROF. This was limited to UNIX platforms and was not always consistent. This PR introduces more ways to schedule the sampling. They are the following:

  • _SigprofScheduler uses signals.SIGPROF to schedule
  • _SigalrmScheduler uses signals.SIGALRM to schedule
  • _SleepScheduler uses threads and time.sleep to schedule
  • _EventScheduler uses threads and threading.Event().wait to schedule

Previously, the only scheduling mechanism was via `signals.SIGPROF`. This was
limited to UNIX platforms and was not always consistent. This PR introduces more
ways to schedule the sampling. They are the following:

- `_SigprofScheduler` uses `signals.SIGPROF` to schedule
- `_SigalrmScheduler` uses `signals.SIGALRM` to schedule
- `_SleepScheduler` uses threads and `time.sleep` to schedule
- `_EventScheduler` uses threads and `threading.Event().wait` to schedule
@Zylphrex
Zylphrex marked this pull request as ready for review September 15, 2022 20:13
@Zylphrex
Zylphrex requested review from a team and sl0thentr0py September 19, 2022 18:05
Comment thread sentry_sdk/profiler.py
while True:
if self.event.is_set():
break
self.event.wait(timeout=self._interval)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only event.set is in stop_profiling so won't this always timeout?

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.

Yes, that's the intent. The idea with this scheduler is to use this timeout mechanism to schedule the next time the sampling function is called. One possible concern with this approach is that CPU usage may spike due to the wait but this will be tested more and verified before we make a final decision.

@sl0thentr0py sl0thentr0py Sep 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah gotcha, thx for explaining!

@Zylphrex
Zylphrex merged commit 19720e6 into master Sep 19, 2022
@Zylphrex
Zylphrex deleted the txiao/feat/introduce-different-profiler-schedulers branch September 19, 2022 20:48
Zylphrex added a commit that referenced this pull request Sep 20, 2022
Zylphrex added a commit that referenced this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants