Implementation of MapBuilder::build() could be more efficient

[sebastianbergmann]

Here is a script for profiling the code in question.

<?php declare(strict_types=1);
use SebastianBergmann\CodeCoverage\Filter;
use SebastianBergmann\CodeCoverage\StaticAnalysis\CachingFileAnalyser;
use SebastianBergmann\CodeCoverage\StaticAnalysis\ParsingFileAnalyser;
use SebastianBergmann\CodeCoverage\Test\Target\MapBuilder;
use SebastianBergmann\FileIterator\Facade;

require __DIR__ . '/vendor/autoload.php';

$filter = new Filter;
$files = (new Facade)->getFilesAsArray(__DIR__ . '/src', '.php');
$filter->includeFiles($files);

$builder = new MapBuilder;

$map = $builder->build(
    $filter,
    new CachingFileAnalyser(
        '/tmp/coverage-cache',
        new ParsingFileAnalyser(true, false),
        true,
        false
    )
);

[sebastianbergmann]

CC @edorian @staabm

[staabm]

team analysis yieled

•  file_get_contents in CachingFileAnalyser is called twice per file
•  md5 for hashing is slower in recent php, because sha256 is optimized on modern cpus (and recent php versions)
•  maybe we can drop a is_file right before realpath, or file_get_contents
•  array_unqiue seems expensive and we maybe can instead write self-de-duplicating array offsets

[staabm]

I had a look for a few hours at the current implementation. Did a few experiments but nothing yielded a meaningful improvement.

either we need a more real world reproducer, or the bottleneck does not kick in as long as the cache is poppulated/warm.

[edorian]

Wrapping my head around this wasn't easy with me being unfamiliar with the code and the variables just being $filename and $directory.

But if I got everything right, we should be able to compute the file hash only once in https://github.com/sebastianbergmann/php-code-coverage/blob/main/src/StaticAnalysis/CachingFileAnalyser.php#L212 and store that in a $filename => hash_file($algom, $filename) lookup, which should get rid of a disk reads at least. If we can assume the source file doesn't change during a coverage run.

Preliminary testing suggests a small speed up in the test case you posted, but I will do some more testing

[sebastianbergmann]

I have uncommitted changes that, once complete, change the API of the static analysis from files to source, reducing the number of file_get_contents() calls to 1 as well as the hashing. Just need to find the time to finish this refactoring.