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

Disable Xdebug in subprocesses when it is not used #5989

Closed
wants to merge 1 commit into from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 13, 2024

before this PR xdebug was only auto-disabled for PHPT.

as of this PR we disable xdebug for all sub-processes to make them faster - but only if no active debugger is attached.

this means subprocess isolated tests can still be debugged without the need for a additional cli-option or config setting changed just by attaching e.g. PHPStorm (so no change in DX).


I initially tried disabling xdebug for the whole sub-process by not loading the extension at all, but this would either require a new composer/xdebug-handler dependency or a way more involved fix.

disabling xdebug already provides most perf benefits.


Benchmark (xdebug loaded, but no debugger attached):

/c/tools/php83/php phpunit tests/end-to-end/data-provider/log-junit-isolation.phpt with phpunit@a82cc0548 takes 2.5 - 2.6 seconds (phpunit reported runtime)

After this PR: /c/tools/php83/php phpunit tests/end-to-end/data-provider/log-junit-isolation.phpt takes 0.5-0.6 seconds

tested with PHP 8.3.11 with Xdebug v3.4.0beta1 on windows11x64

php.ini settings:

zend_extension=xdebug
xdebug.mode=debug
xdebug.start_with_request=yes

@staabm staabm marked this pull request as ready for review October 13, 2024 09:29
public function testOne(): void
{
$this->assertTrue(extension_loaded('xdebug'));
$this->assertSame('', (string) ini_get('xdebug.mode'));
Copy link
Contributor Author

@staabm staabm Oct 13, 2024

Choose a reason for hiding this comment

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

when -d xdebug.mode=off is passed, xdebug.mode returns '':

➜  phpunit git:(xdebugsub) ✗ php -d 'xdebug.mode=off' test.php
string(0) ""
➜  phpunit git:(xdebugsub) ✗ php -d 'xdebug.mode=coverage' test.php
string(8) "coverage"
➜  phpunit git:(xdebugsub) ✗ cat test.php
<?php

var_dump((string) ini_get('xdebug.mode'));

@staabm staabm force-pushed the xdebugsub branch 2 times, most recently from 6c71e7f to 1090876 Compare October 13, 2024 09:38
Copy link

codecov bot commented Oct 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.67%. Comparing base (a82cc05) to head (5062ac3).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5989   +/-   ##
=========================================
  Coverage     94.67%   94.67%           
- Complexity     6406     6408    +2     
=========================================
  Files           694      694           
  Lines         19189    19191    +2     
=========================================
+ Hits          18167    18169    +2     
  Misses         1022     1022           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@staabm staabm force-pushed the xdebugsub branch 3 times, most recently from 60ecc0f to e4251bb Compare October 13, 2024 10:12
@sebastianbergmann sebastianbergmann added feature/test-runner CLI test runner type/performance Issues related to resource consumption (time and memory) labels Oct 13, 2024
@staabm
Copy link
Contributor Author

staabm commented Oct 14, 2024

I think this one is now good to go

@@ -0,0 +1,38 @@
--TEST--
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should this test be moved in a different folder?

@sebastianbergmann sebastianbergmann changed the base branch from main to 11.4 October 18, 2024 10:10
@sebastianbergmann sebastianbergmann changed the base branch from 11.4 to main October 18, 2024 10:11
@sebastianbergmann sebastianbergmann changed the title Disable xdebug in subprocess when inactive Disable Xdebug in subprocesses when it is not used Oct 18, 2024
@sebastianbergmann
Copy link
Owner

Cherry-picked into 11.4 and merged to 11.5 and main from there. Thanks!

@staabm staabm deleted the xdebugsub branch October 18, 2024 11:10
@staabm
Copy link
Contributor Author

staabm commented Oct 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-runner CLI test runner type/performance Issues related to resource consumption (time and memory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants