Speed improvements when using xdebug

[MajorCaiger]

• Init data by running xdebug with 'unused' and 'dead code' flags on all whitelist files
• Run xdebug without flags for each test method
• Cache num lines check for each file, to prevent superflous checks

[sebastianbergmann]

Thank you for your contribution. Do you have some performance number you can share?

[MajorCaiger]

Hi all.

This is my first time contributing to this project.
I would love some feedback on these changes.

These changes are due to the following. During the execution of each test, xdebug is run with 'unused' and 'dead code' flags on, which mean each file that is touched as a part of that test execution (including vendor classes, phpunit class, mockery class and other classes outside of the whitelist) get parsed by xdebug which slows everything down. These changes ensure that only whitelisted files are run through xdebug with those flags on, then for each test xdebug runs without flags to determine the actual executed lines.

Here is a snippet of the phpunit output on a project that I am currently working on

Time: 11.85 minutes, Memory: 479.75Mb

OK, but incomplete, skipped, or risky tests!
Tests: 4366, Assertions: 40451, Skipped: 100, Incomplete: 5.

Generating code coverage report in HTML format ... done

real    12m24.480s
user    12m19.608s
sys 0m4.796s

Here is a snippet of the output after the changes

Time: 1.45 minutes, Memory: 482.75Mb

OK, but incomplete, skipped, or risky tests!
Tests: 4366, Assertions: 40451, Skipped: 100, Incomplete: 5.

Generating code coverage report in HTML format ... done

real    2m4.492s
user    1m59.886s
sys 0m3.745s

As you can see there is a massive speed boost with these changes.

[MajorCaiger]

My concerns with this pull request are that

• I have had to alter the driver interface, which would be a BC break if anyone had created custom drivers.
• I have essentially removed the use for the 'processUncoveredFilesFromWhitelist' flag

I would be grateful of any suggestions that would allow this concept to be applied?

[sebastianbergmann]

I am definitely interested in these changes, especially since the numbers you provided in #384 (comment) look very promising.

However, even without the changes to the driver interface this patch would be outside the scope of a bugfix release for the 2.2 branch. This will have to go into master for 3.0.

[MajorCaiger]

Understood.
I used 2.2 as a starting point as I don't have a php 5.6 environment set up. I will set myself up a php 5.6 environment and re-submit to 3.0 over the next couple of days.

[sebastianbergmann]

No worries, I am reviewing your patch right now and may be able to cherry-pick it into master later today.

[sebastianbergmann]

Overall, the patch looks good to me. However, I am not (yet) comfortable with the unconditional include_once statement in the PHP_CodeCoverage::initData() method. This operation was previously controlled by the now deprecated/removed flag in the $processUncoveredFilesFromWhitelist attribute.

There must be a way to prevent the inclusion of uncovered files for legacy code that contains files that not only declare classes, interfaces, traits, or functions but also contain executable code in their global scope. An alternative could be to suggest not adding such problematic files to the whitelist.

What do you think?

[MajorCaiger]

I seem to remember that without the processUncoveredFilesFromWhitelist flag, it used to ignore the uncovered files, but if you look at the large chunk of code I removed, when the flag is false it appears to grab the number of lines from the file and mark then all as not executed. Looking at the code, it seems the only difference is that with flag $processUncoveredFilesFromWhitelist=true, you find out about dead code, and it only marks executable lines as uncovered, whereas $processUncoveredFilesFromWhitelist=false shows all lines as uncovered (even non-executable lines) unless I am reading the code wrong?

[sebastianbergmann]

Do your changes also work in case no whitelist is used?

[MajorCaiger]

Good question. Probably not. I will do a little testing over the next few days and let you know my findings.

[sebastianbergmann]

Before your patch, PHPUnit's addUncoveredFilesFromWhitelist="true" tells PHP_CodeCoverage to add all whitelisted files to the report. Uncovered files are shown with all their lines as not executable unless PHPUnit's processUncoveredFilesFromWhitelist="true" is used.

[sebastianbergmann]

On the https://github.com/sebastianbergmann/php-code-coverage/tree/speedup you can find my "cleaned up" version of your changes so far. Please use this as the basis for further work. Thanks!

[MajorCaiger]

OK Thanks.

I'll take a look and get back to you.

[MajorCaiger]

Hi @sebastianbergmann

Are you able to give me access to the speedup branch so that I can push my changes directly to it? Or shall I submit another merge request?

There were a few issues around lines that should be ignored but were being shown as uncovered which I have fixed. This actually increased speed and reduced memory usage a little more to.

I have had to removed references to processUncoveredFilesFromWhitelist on a branch of phpunit which I can also submit if it is useful?

[sebastianbergmann]

You should be able to push now.