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

Fixed issue #1755: Overload pcntl_fork() to prevent performance degradation by calling xdebug_get_pid often #556

Closed

Conversation

@carlos-granados
Copy link
Contributor

carlos-granados commented Mar 5, 2020

In version 2.7 a change was introduced to restart the debugger if the process id changed. This was added to fix issue # 938 which stopped you from correctly debugging forked processes after using pnctl_fork() . The problem is that now we are calling xdebug_get_pid in a lot of places and this slows down the debugger a lot. It is not actually necessary to be calling xdebug_get_pid continually as we can just do that when the process is forked and restart the debugger at that point if needed. We do this by overriding the pnctl_fork() php function and checking the process id and restarting the debugger if needed there

By doing this, the measured time to run this php code when connected to a debugging client

<?php
function fib($n)
{
    if ($n <= 1) {
        return 1;
    }
    return fib($n - 1) + fib($n - 2);
}

echo fib(35);

decreased from 2:16 min to 0:59min

Copy link
Contributor

derickr left a comment

Hi,

thanks for this. I have a few comments.

Besides the comments, would you also please use the correct commit message format? All fixes in Xdebug with links to the issue tracker are in the following form:

Fixed issue #1755: Overload pcntl_fork() to prevent performance degradation by calling xdebug_get_pid often

I prefer my commit messages (and Issue Summary) to be an active change and say what it improved, and not what was broken before. It makes for a much easier to read change log.

src/base/base.c Outdated Show resolved Hide resolved
src/base/base.c Outdated Show resolved Hide resolved
src/debugger/com.c Outdated Show resolved Hide resolved
src/debugger/com.c Outdated Show resolved Hide resolved
xdebug.c Show resolved Hide resolved
xdebug.c Outdated Show resolved Hide resolved
@carlos-granados carlos-granados force-pushed the i6systems:issue1755-optimize-pid-checks branch from edcb4cd to a6738b4 Mar 6, 2020
@carlos-granados carlos-granados changed the title Issue 1755 - Only check the pid and restart the debugger when running pcntl_fork Fixed issue #1755: Overload pcntl_fork() to prevent performance degradation by calling xdebug_get_pid often Mar 6, 2020
@carlos-granados

This comment has been minimized.

Copy link
Contributor Author

carlos-granados commented Mar 6, 2020

Did all requested changes, thanks for your helpful comments. I am going to be away and unavailable until next Friday so I will not be able to answer any further questions or requests until then

derickr added a commit that referenced this pull request Mar 28, 2020
@derickr

This comment has been minimized.

Copy link
Contributor

derickr commented Mar 28, 2020

I've merged this after a rebase and a few tweaks. Thanks! It's going to make it into Xdebug 3.0.

@derickr derickr closed this Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.