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 thread names #2934

Merged
merged 16 commits into from
Nov 12, 2024
Merged

feat(profiling) add thread names #2934

merged 16 commits into from
Nov 12, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Nov 7, 2024

Description

This PR will add a thread name label to collected samples. In most cases a dedicated thread name is either not set or even if it is not helpful. For NTS PHP we do just use the SAPI name as the thread name, this seems helpful. For ZTS we currently only collect a thread name for FrankenPHP as it sets meaningful names.
Additional notes:

  • on Darwin a thread name might be empty if not set
  • on Linux a thread name is the process name if not explicitly set (which is the case for most)

In the future we should try and detect Apache and parallel threads and name them accordingly.

PROF-10863

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing labels Nov 7, 2024
@realFlowControl realFlowControl self-assigned this Nov 7, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.97%. Comparing base (84f7fa8) to head (1b4be1c).
Report is 9 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (84f7fa8) and HEAD (1b4be1c). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (84f7fa8) HEAD (1b4be1c)
tracer-php 12 11
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2934      +/-   ##
============================================
- Coverage     82.10%   73.97%   -8.13%     
  Complexity     2527     2527              
============================================
  Files           108      108              
  Lines         10360    10360              
============================================
- Hits           8506     7664     -842     
- Misses         1854     2696     +842     
Flag Coverage Δ
tracer-php 73.97% <ø> (-8.13%) ⬇️

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

see 8 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 84f7fa8...1b4be1c. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Nov 7, 2024

Benchmarks [ profiler ]

Benchmark execution time: 2024-11-12 08:02:08

Comparing candidate commit 1b4be1c in PR branch florian/thread-name with baseline commit 84f7fa8 in branch master.

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

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

  • 🟥 cpu_user_time [+35.245ms; +91.278ms] or [+2.645%; +6.850%]

@realFlowControl realFlowControl force-pushed the florian/thread-name branch 4 times, most recently from d5f66f6 to 2b2115b Compare November 8, 2024 11:01
@realFlowControl realFlowControl marked this pull request as ready for review November 8, 2024 13:26
@realFlowControl realFlowControl requested review from a team as code owners November 8, 2024 13:26
profiling/src/profiling/thread_utils.rs Outdated Show resolved Hide resolved
profiling/src/profiling/thread_utils.rs Outdated Show resolved Hide resolved
@realFlowControl realFlowControl merged commit 49b3dc0 into master Nov 12, 2024
680 of 726 checks passed
@realFlowControl realFlowControl deleted the florian/thread-name branch November 12, 2024 15:24
@github-actions github-actions bot added this to the 1.5.0 milestone Nov 12, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants