Huge memory leak in PHPUnit TypeInferenceTest when using data providers

[gnutix]

Hello there,

I've noticed today that our project's test suite failed locally with a memory exhausted error, even though we run it with 4 GB (:speak_no_evil:).

I've narrowed it down to an implementation of a PHPStan's TypeInferenceTest that covers the types of our Functional library. It uses a data provider that loads 31 PHP files, containing a total of 161 calls to PHPStan's assertType.

After a bit of fiddling, I tried removing the data provider and loading the 31 files directly in the test method using a foreach, and here's the difference :

With the data provider :

Time: 02:37.201, Memory: 5.44 GB

OK (169 tests, 336 assertions)

Without :

Time: 00:01.392, Memory: 118.50 MB

OK (1 test, 336 assertions)

So, I created two Blackfire profiles, and it seems like an almost infinite loop, looking at the number of calls to SebastianBergmann\Exporter\Exporter::recursiveExport. My dependencies are phpunit/phpunit: 10.3.5 and sebastian/exporter: 5.1.0.

Test code before :

    /**
     * @return iterable<mixed>
     */
    public static function dataAsserts(): iterable
    {
        foreach ((new Finder())->in(__DIR__ . '/dataproviders')->files()->name('*.php') as $file) {
            yield from self::gatherAssertTypes($file->getRealPath());
        }
    }

    #[DataProvider('dataAsserts')]
    public function testAsserts(string $assertType, string $file, mixed ...$args): void
    {
        $this->assertFileAsserts($assertType, $file, ...$args);
    }

Test code after :

    public function testAsserts(): void
    {
        foreach ((new Finder())->in(__DIR__ . '/dataproviders')->files()->name('*.php') as $file) {
            /** @var array{0: string, 1: string, 2: array<mixed>} $args */
            foreach (self::gatherAssertTypes($file->getRealPath()) as $args) {
                $this->assertFileAsserts(...$args);
            }
        }
    }

I'm not sure if I should open an issue on PHPUnit, Exporter or here, so I'll start here. 🤷‍♂️
Reproducing repository : https://github.com/gnutix/phpstan-typeinferencetest-memory-exhausted

gnutix

[gnutix]

/cc @sebastianbergmann You might be interested by this.

[staabm]

could you provide a reproducing repo?

[gnutix]

@staabm There you go : https://github.com/gnutix/phpstan-typeinferencetest-memory-exhausted

[sebastianbergmann]

This is expected, at least to me. The data provided to a test by a data provider is exported to a string for the value object that represents the test for the event system. If you use data providers to create object graphs that are expensive to export then yes, this will be slow. Ideally, data providers should only provide scalar values or "small" value objects. If your test fixture has to be a large object graph, then construct that in your test method from the aforementioned scalar values or "small" value objects.

[sebastianbergmann]

Ideally PHPUnit could yield a single value from a dataProvider, call the test methods with these arguments, and throw it away.

Ideally, yes. We are just not there yet. We are working in incremental steps towards a future where this will be possible. The fact that we were "too greedy" when we implemented the value object that represents a test is unrelated to how data providers currently work. PHPUnit 10.4 addresses the former, not the latter.

@staabm Could you check whether sebastianbergmann/phpunit#5524 improves the situation? Thanks!

[sebastianbergmann]

Beat me to it, thanks :)

[ondrejmirtes]

Thank you very much, the progress is awesome 😊

[staabm]

in case its still relevant, I tried phpunit 10 sebastianbergmann/phpunit@cf5af61 (so without the latest fix for sebastianbergmann/phpunit#5524) and its super slow.

-> the fix for sebastianbergmann/phpunit#5524 brings performance back into phpunit9 levels for our case at hand

thank you

[gnutix]

Thanks to all of you for your involvement 🙏

[gnutix]

So I guess a better middle ground here would be to load the 31 files paths in the data provider, then use gatherAssertTypes in the test to do each assertions ?

I wouldn't have thought that the object graph would be that big.. but I didn't know that all of it was converted into a string either. Thanks for the info.

[staabm]

I was wondering why the NodeScopeResolverTest in PHPStan itself does not benefit the same way from the fix the repro repository does.

I think the main problem of the repro repo is, that the tests don't wrap the assertType calls in a callable, therefore the code is executed multiple times

gnutix/phpstan-typeinferencetest-memory-exhausted@2a4c462#diff-6d43b801e01f05918cb84770aa7fcf3ebc3625b545db208ad5405f61407a1b77

e.g.

<?php

/** @noinspection PhpExpressionResultUnusedInspection */
declare(strict_types=1);

namespace Tests\dataproviders;

use function App\Functional\pairs;
use function PHPStan\Testing\assertType;

/**
 * @var list<int> $list
 * @var array{id: int, name: string} $map
 * @var array{0: int, 1: string} $tuple
 * @var list<string> $listWithNumericKeys
 * @var array<string, string> $arrayWithStringKeys
 */
assertType('list<array{*NEVER*, *NEVER*}>', pairs([]));
assertType('list<array{*NEVER*, *NEVER*}>', pairs([], [], []));
assertType('list<array{int, int}>', pairs($list));
assertType('list<array{int, string}>', pairs($listWithNumericKeys));
assertType('list<array{string, string}>', pairs($arrayWithStringKeys));
assertType('list<array{string, int|string}>', pairs($map));
assertType('list<array{int, int|string}>', pairs($tuple));

should be

<?php

/** @noinspection PhpExpressionResultUnusedInspection */
declare(strict_types=1);

namespace Tests\dataproviders;

use function App\Functional\pairs;
use function PHPStan\Testing\assertType;

function doFoo() {
	/**
	 * @var list<int> $list
	 * @var array{id: int, name: string} $map
	 * @var array{0: int, 1: string} $tuple
	 * @var list<string> $listWithNumericKeys
	 * @var array<string, string> $arrayWithStringKeys
	 */
	assertType('list<array{*NEVER*, *NEVER*}>', pairs([]));
	assertType('list<array{*NEVER*, *NEVER*}>', pairs([], [], []));
	assertType('list<array{int, int}>', pairs($list));
	assertType('list<array{int, string}>', pairs($listWithNumericKeys));
	assertType('list<array{string, string}>', pairs($arrayWithStringKeys));
	assertType('list<array{string, int|string}>', pairs($map));
	assertType('list<array{int, int|string}>', pairs($tuple));
}

instead

[staabm]

just confirmed, wrapping the assertType() calls into a function makes the memory-consumption and runtime massively faster.

[ondrejmirtes]

The test file actually gets executed? Why?

[staabm]

I don't know why. I just wrapped the existing tests in a function and metrics got waaay better

PHPUnit 10.4-dev by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.10
Configuration: /Users/staabm/workspace/memopt/phpunit.xml.dist

..F.FFF.FFFFFF....F.FFFFFFFFFF                                    30 / 30 (100%)

Time: 00:00.077, Memory: 72.00 MB

instead of 530 MB

[ondrejmirtes]

That should be investigated.

[ondrejmirtes]

Also, as an alternative, you can analyze the dataproviders directory with PHPStan, instead of running PHPUnit tests, it would also tell you about failing assertions, and I guess the memory leak isn't going to be an issue :)

[gnutix]

When should one be used over the other then ?

[ondrejmirtes]

You can use either of them according to your preference. Do you want to run vendor/bin/phpunit and get failures, or vendor/bin/phpstan 😊

[ondrejmirtes]

Please note that this is no longer a problem as of PHPStan 1.10.66 thanks to this fix: phpstan/phpstan-src@da87a65

So make sure you've updated to this version. Your test suite should be fast even with PHPUnit 10 and 11.