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

WIP: feat(profiling): track profiler overhead #2305

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

realFlowControl
Copy link
Member

Description

This PR aims to track the overhead of profiling.
This is currently in PoC phase 😉

Readiness checklist

  • (only for Members) Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

Copy link
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Why is there a new sample type rather than putting them into timeline?

@realFlowControl
Copy link
Member Author

realFlowControl commented Oct 6, 2023

I'm just playing with ideas currently. Instead of adding a new sample for every sample that we create we might be able to attach to the sample currently created, which means we are not doubling the number of samples. It has some drawbacks, as we do not measure the entire round trip and we are for example missing the time it takes to try_send() the sample to the other thread.
We are btw. still able to surface these in the timeline, even when the times are in another sample type

@pr-commenter
Copy link

pr-commenter bot commented Nov 8, 2023

Benchmarks

Benchmark execution time: 2023-11-08 15:47:48

Comparing candidate commit 78bce3b in PR branch florian/track-overhead with baseline commit a1f35f6 in branch master.

Found 0 performance improvements and 2 performance regressions! Performance is the same for 16 metrics, 3 unstable metrics.

scenario:php-profiler-timeline-memory-with-profiler-and-timeline

  • 🟥 max_rss_usage [+1.468MB; +1.764MB] or [+5.064%; +6.086%]

scenario:walk_stack/1

  • 🟥 wall_time [+280.727ns; +284.433ns] or [+2.310%; +2.341%]

@realFlowControl realFlowControl changed the title WIP: track profiler overhead WIP: feat(profiling): track profiler overhead Nov 9, 2023
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