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) add stream_select()-type functions to timeline #2943

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Nov 12, 2024

Description

This will wrap functions that are used to wait on blocking I/O, like stream_select() and others. It also moves the frankenphp_handle_request() to the idle visualisation because technically it is waiting on I/O, but it is used to wait for the next request, just like RSHUTDOWN -> RINIT so it should share the same vis.

In async PHP (using ReactPHP, AMPHP or others), spending time in a stream_select()-type function call means the event-loop is idle.

PPROF-10884

AMPHP event loop:
image

ext-parallel's \parallel\Events::poll() method:
image

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing labels Nov 12, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.97%. Comparing base (3abddef) to head (31a763d).
Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2943      +/-   ##
============================================
+ Coverage     72.46%   73.97%   +1.51%     
  Complexity     2527     2527              
============================================
  Files           135      108      -27     
  Lines         14402    10360    -4042     
  Branches        991        0     -991     
============================================
- Hits          10436     7664    -2772     
+ Misses         3422     2696     -726     
+ Partials        544        0     -544     
Flag Coverage Δ
appsec-extension ?
tracer-php 73.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 27 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3abddef...31a763d. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Nov 12, 2024

Benchmarks [ profiler ]

Benchmark execution time: 2024-11-13 14:53:24

Comparing candidate commit 31a763d in PR branch florian/event-loop-viz with baseline commit 3abddef in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 9 unstable metrics.

@realFlowControl realFlowControl changed the title feat(profiling) add select()-type functions to timeline feat(profiling) add stream_select()-type functions to timeline Nov 12, 2024
@realFlowControl realFlowControl self-assigned this Nov 12, 2024
@realFlowControl realFlowControl marked this pull request as ready for review November 12, 2024 14:01
@realFlowControl realFlowControl requested a review from a team as a code owner November 12, 2024 14:01
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.

First glance looks good, will look a bit more later!

@realFlowControl realFlowControl added this to the 1.5.0 milestone Nov 13, 2024
profiling/src/timeline.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot removed this from the 1.5.0 milestone Nov 13, 2024
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.

I don't have any other feedback. Looks good to me.

@realFlowControl realFlowControl merged commit ba07209 into master Nov 14, 2024
699 of 726 checks passed
@realFlowControl realFlowControl deleted the florian/event-loop-viz branch November 14, 2024 07:01
@github-actions github-actions bot added this to the 1.5.0 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
profiling Relates to the Continuous Profiler tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants