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) PHP ZTS support for CPU- and walltime profiling #2470

Merged
merged 19 commits into from
Feb 14, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Jan 15, 2024

Description

This PR will:

  • make CPU- and Walltime profiling work in PHP ZTS
  • build ZTS profiler in CI and add to the artefact we ship
  • run tests for ZTS version
  • disable allocation profiling in ZTS (would crash otherwise and will be shipped later)

PROF-8921

#2070

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing labels Jan 15, 2024
@pr-commenter
Copy link

pr-commenter bot commented Jan 15, 2024

Benchmarks

Benchmark execution time: 2024-02-06 18:12:51

Comparing candidate commit c193abb in PR branch florian/cpu-time-zts with baseline commit 27f4013 in branch master.

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

@realFlowControl realFlowControl force-pushed the florian/cpu-time-zts branch 4 times, most recently from cb0b81d to e6b800d Compare January 29, 2024 16:24
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024

Codecov Report

Merging #2470 (f1ddfed) into master (27f4013) will increase coverage by 2.07%.
The diff coverage is n/a.

❗ Current head f1ddfed differs from pull request most recent head c193abb. Consider uploading reports for the commit c193abb to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2470      +/-   ##
============================================
+ Coverage     76.53%   78.61%   +2.07%     
+ Complexity      267      216      -51     
============================================
  Files           136      105      -31     
  Lines         17526    12936    -4590     
  Branches       1020        0    -1020     
============================================
- Hits          13413    10169    -3244     
+ Misses         3580     2767     -813     
+ Partials        533        0     -533     
Flag Coverage Δ
appsec-extension ?
tracer-extension 78.54% <ø> (+0.05%) ⬆️
tracer-integrations 79.86% <ø> (+0.37%) ⬆️

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

see 59 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 27f4013...c193abb. Read the comment docs.

@realFlowControl realFlowControl self-assigned this Jan 30, 2024
@realFlowControl realFlowControl marked this pull request as ready for review January 30, 2024 14:19
@realFlowControl realFlowControl requested review from a team as code owners January 30, 2024 14:19
profiling/build.rs Outdated Show resolved Hide resolved
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.

A few things to manually check:

  • Does datadog-setup.php need any changes? Based on how it detects support, it should Just Work but we may need to change comments or output or something.
  • Have you checked Apache in the worker modes that support threading? I'd also double-check apachectl graceful.

Aside from these manual testing concerns, it looks fine to me.

@realFlowControl realFlowControl changed the title feat(profiling) ZTS CPU-/Walltime profiling feat(profiling) PHP ZTS support for CPU- and walltime profiling Feb 14, 2024
@realFlowControl realFlowControl merged commit d8a8624 into master Feb 14, 2024
610 of 612 checks passed
@realFlowControl realFlowControl deleted the florian/cpu-time-zts branch February 14, 2024 16:07
@github-actions github-actions bot added this to the 0.98.0 milestone Feb 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants