Skip to content

Loader: disable the tracer in case of a potential incompatibility (extension/jit)#2853

Merged
iamluc merged 8 commits intomasterfrom
luc/loader-incompat-extensions
Sep 27, 2024
Merged

Loader: disable the tracer in case of a potential incompatibility (extension/jit)#2853
iamluc merged 8 commits intomasterfrom
luc/loader-incompat-extensions

Conversation

@iamluc
Copy link
Copy Markdown
Contributor

@iamluc iamluc commented Sep 16, 2024

Description

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 16, 2024 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.41%. Comparing base (3746a2f) to head (98fc95f).
⚠️ Report is 1018 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2853      +/-   ##
============================================
+ Coverage     78.39%   78.41%   +0.01%     
  Complexity     2526     2526              
============================================
  Files           173      173              
  Lines         18710    18713       +3     
  Branches        976      976              
============================================
+ Hits          14667    14673       +6     
+ Misses         3506     3503       -3     
  Partials        537      537
Flag Coverage Δ
appsec-extension 69.08% <ø> (ø)
tracer-extension 78.26% <100.00%> (+0.08%) ⬆️
tracer-php 82.10% <ø> (ø)

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

Files with missing lines Coverage Δ
zend_abstract_interface/hook/hook.c 54.98% <100.00%> (+0.55%) ⬆️

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 3746a2f...98fc95f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@iamluc iamluc marked this pull request as ready for review September 16, 2024 13:27
@iamluc iamluc requested a review from a team as a code owner September 16, 2024 13:27
@iamluc iamluc marked this pull request as draft September 16, 2024 14:32
bwoebi
bwoebi reviewed Sep 16, 2024
View reviewed changes
Comment thread loader/dd_library_loader.c Outdated
@iamluc iamluc force-pushed the luc/loader-incompat-extensions branch from 5d19319 to 8b50c15 Compare September 17, 2024 08:45
@iamluc iamluc changed the title Loader: don't inject the tracer if an incompatible extention is detected Loader: don't inject the tracer if an incompatible extention is detected or JIT enabled Sep 17, 2024
@iamluc iamluc force-pushed the luc/loader-incompat-extensions branch from 8b50c15 to cd64269 Compare September 17, 2024 09:32
@iamluc iamluc changed the title Loader: don't inject the tracer if an incompatible extention is detected or JIT enabled Loader: disable the tracer in case of a potential incompatibility (extension/jit) Sep 18, 2024
@iamluc iamluc force-pushed the luc/loader-incompat-extensions branch from 28513da to 7d16a17 Compare September 18, 2024 07:52
bwoebi
bwoebi reviewed Sep 18, 2024
View reviewed changes
Comment thread loader/dd_library_loader.c Outdated
@iamluc iamluc force-pushed the luc/loader-incompat-extensions branch 3 times, most recently from 7240136 to 7fa7896 Compare September 18, 2024 13:40
@iamluc iamluc marked this pull request as ready for review September 18, 2024 14:43
bwoebi
bwoebi reviewed Sep 19, 2024
View reviewed changes
Comment thread loader/dd_library_loader.c
bwoebi
bwoebi reviewed Sep 19, 2024
View reviewed changes
Comment thread loader/valgrind.supp Outdated
@iamluc iamluc force-pushed the luc/loader-incompat-extensions branch from 7fa7896 to 98fc95f Compare September 19, 2024 16:01
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Sep 19, 2024

Benchmarks [ tracer ]

Benchmark execution time: 2024-09-19 16:25:56

Comparing candidate commit 98fc95f in PR branch luc/loader-incompat-extensions with baseline commit 3746a2f in branch master.

Found 5 performance improvements and 4 performance regressions! Performance is the same for 169 metrics, 0 unstable metrics.

scenario:LaravelBench/benchLaravelBaseline

  • 🟥 execution_time [+62.126µs; +221.334µs] or [+2.133%; +7.600%]

scenario:LaravelBench/benchLaravelOverhead

  • 🟥 execution_time [+68.823µs; +210.117µs] or [+2.180%; +6.656%]

scenario:PDOBench/benchPDOBaseline

  • 🟩 execution_time [-14.046µs; -11.164µs] or [-7.277%; -5.784%]

scenario:PDOBench/benchPDOBaseline-opcache

  • 🟥 execution_time [+12.831µs; +14.077µs] or [+7.147%; +7.841%]

scenario:PDOBench/benchPDOOverhead

  • 🟩 execution_time [-14.536µs; -13.125µs] or [-5.080%; -4.587%]

scenario:PDOBench/benchPDOOverhead-opcache

  • 🟥 execution_time [+12.594µs; +14.664µs] or [+4.355%; +5.070%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4

  • 🟩 execution_time [-550.796ns; -235.604ns] or [-7.396%; -3.163%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache

  • 🟩 execution_time [-517.820ns; -164.580ns] or [-7.234%; -2.299%]

scenario:WordPressBench/benchWordPressOverhead

  • 🟩 execution_time [-21.840ms; -21.611ms] or [-82.938%; -82.071%]

bwoebi
bwoebi approved these changes Sep 27, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks :-)

@iamluc iamluc merged commit 9d04195 into master Sep 27, 2024
@iamluc iamluc deleted the luc/loader-incompat-extensions branch September 27, 2024 13:21
@github-actions github-actions Bot added this to the 1.4.0 milestone Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bwoebi bwoebi bwoebi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@iamluc @codecov-commenter @bwoebi