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

Memory leaks in PHPUnit #30736

Closed
brendt opened this issue Dec 3, 2019 · 30 comments
Closed

Memory leaks in PHPUnit #30736

brendt opened this issue Dec 3, 2019 · 30 comments

Comments

@brendt
Copy link
Contributor

brendt commented Dec 3, 2019

  • Laravel Version: v6.6.0
  • PHP Version: 7.4.0 and 7.3
  • Database Driver & Version: sqlite 3.24.0 and MySQL 5.7.25

Description:

Recently we noticed our testsuite throwing out of memory errors, a problem that hadn't happened before. Doing a little debugging we found that each test adds around 2 MB of memory usage. Our test suite has around 1000 tests, and to have it fully run we need around 600 MB of memory allocated.

Around 80% of our test cases are "unit" tests, though they do interact with the database, 20% are integration tests that will simulate requests to controllers, using the built-in Laravel functionality.

We're using both the CreatesApplication and RefreshDatabase traits as provided by Laravel, and use an in-memory sqlite database, however for this issue I also ran our test suite on a MySQL database, where the same leak seems to happen.

I have looked at several older issues and stackoverflow questions, though none of them seem to provide a solution that works in this case.

Steps To Reproduce:

Add this teardown function in the base testcase, and observe.

    protected function tearDown(): void
    {
        parent::tearDown();

        echo memory_get_usage() . PHP_EOL;
    }

Output:

.57536280
.59370792
.63121592
.65270952
…
.83115760
.84632896
.85978784
.82666928
…
@brendt brendt changed the title Memory leaks in PhpUnit Memory leaks in PHPUnit Dec 3, 2019
@gocanto
Copy link
Contributor

gocanto commented Dec 3, 2019

Easy way for you to handle this would extending directly from PHPUnit and letting the framework aside. Hence, you will indeed remove these issues since DB is not in place anymore.

Now, if you wanna have DB interaction just for the peace of mind, I would suggest moving them as feature. Once they are there, create two test suits. One for Unit and One for feature and start moving tests from feature to unit.

This is not an answer for your issue, but I would start from there since I had to do the same in the past once you hit a big number of tests.

@mfn
Copy link
Contributor

mfn commented Dec 3, 2019

I'm still on 5.8 using 7.3 but I was curious and ran a private test suite with >6k tests (absolutely mixed: unit, integration, lots of DB) with your tearDown hack and all I saw is just numbers going up.

Actually the test suite is split into two pipelines 5k vs. 1.5k and the memory goes up from 104_318_304 to 458_096_144 (over 5k tests) and from 67_790_784 to 209_527_416 (1k5 tests).

I'm using a 3rd party service (Codeship) and the machine probably has enough RAM so I never noticed this.

My point: "probably has been that way" already, before you discovered it? Or can you confirm it's not happening with 5.8 for you?

Btw. this is with phpunit 8.3.x

@brendt
Copy link
Contributor Author

brendt commented Dec 3, 2019

Or can you confirm it's not happening with 5.8 for you

Also happens on 5.8

@deleugpn
Copy link
Contributor

deleugpn commented Dec 3, 2019

  • Laravel Version: v6.6.0
  • PHP Version: 7.2.19
  • Database Driver & Version: MySQL 5.6.45

I don't know if it's relevant. I ran my test suite in a project with 650 tests, pretty much all of them touching the database, but I'm using actual MySQL and not SQLite.
The first test output was: 39'420'296
The last test output was: 90'611'232

I noticed that it doesn't constantly increase, e.g.

.48095376
.48720792
.49078208
.42856792
.43272440
...
.50419648
.50762032
.51165152
.45723920
.46062656

@brendt
Copy link
Contributor Author

brendt commented Dec 3, 2019

I've also noticed that sometimes memory usage is decreased, though overall it increases throughout the whole run.

@mfn
Copy link
Contributor

mfn commented Dec 3, 2019

I've also noticed that sometimes memory usage is decreased, though overall it increases throughout the whole run.

Same for me. Btw I'm using Postgres for the DB tests.

@driesvints
Copy link
Member

Can we maybe create a simple test case which can reproduce this?

@mfn
Copy link
Contributor

mfn commented Dec 3, 2019

Can we maybe create a simple test case which can reproduce this?

Here we go: https://github.com/mfn/laravel-tests-memleak

Upon checkout:

  • composer install
  • php artisan generateHorde 50
  • vendor/bin/phpunit
    Observe ever increasing memory

A bare bones installation with only adjustments for creating senseless tests. No database involved.

I just remembered some other issue/PR which talked about memory leaks where the culprit was some, I think, closure/reference related leak (but AFAIK this had to do with the Query builder or Eloquent, which doesn't apply in this case).

Or maybe it's such kind of issue with how the application is booted for the tests.

@staudenmeir
Copy link
Contributor

staudenmeir commented Dec 3, 2019

This is a PHPUnit issue. The memory also increases if you remove Laravel from tests/TestCase.php:

<?php

namespace Tests;

abstract class TestCase extends \PHPUnit\Framework\TestCase
{
    //use CreatesApplication;

    protected function tearDown(): void
    {
        parent::tearDown();

        echo memory_get_usage() . PHP_EOL;
    }
}

It looks like the issue was introduced in PHPUnit 8.4. The memory also increases with 8.3, but not as much as with 8.4.

Probably related: sebastianbergmann/phpunit#3915

@dwightwatson
Copy link
Contributor

I noticed that my memory usage increased after upgrading to PHP 7.4.0 - suddenly started hitting the default 128 MB limit. I did the same thing with memory_get_usage() but put it down to being an issue on my end, so glad to see it's not just me.

For reference: Laravel 6.6.1, PHP 7.4.0, Postgres 12.1.

I just tested against PHPUnit 7 as well to compare - similar memory creep but to a lesser extent.

PHPUnit 7.5.17: 137.00 MB
PHPUnit 8.4.3: 165.00 MB

@brendt
Copy link
Contributor Author

brendt commented Dec 4, 2019

@sebastianbergmann could it be that this is a PHPUnit issue and not a Laravel one?

@driesvints
Copy link
Member

I'm gonna leave this open for a bit to follow up but it seems it's best to focus the discussion on the PHPUnit issue linked above by @staudenmeir.

@GrahamCampbell
Copy link
Member

Could you provide your composer.lock file please. There is a known memory leak issue in Mockery 1.3.0. Does upgrading to 1.3.1 do anything?

@dwightwatson
Copy link
Contributor

I don't have access to the same computer as I did earlier, but running the same test suite with Mockery 1.3.1 has the same memory usage.

PHPUnit 8.5.1: 165.00 MB

@brendt
Copy link
Contributor Author

brendt commented Jan 10, 2020

It's still broken with mockery 1.3.1

This is the memory usage of my testsuite, running 1142 tests

Screen Shot 2020-01-10 at 10 00 09

@driesvints
Copy link
Member

@brendt shouldn't we try to bring this to the attention of @sebastianbergmann and @robertbasic? I don't think there's much we can do ourselves here?

@robertbasic
Copy link

We do have some memory leaks in Mockery currently, but we're pretty sure they are related to mocking Demeter chains. Haven't seen anyone here mentioning demeter chains, so I guess then it's not Mockery's fault? Unless Laravel does something under the hood with mocking demeter chains with mockery. I'm not familiar with Laravel at all.

@brendt
Copy link
Contributor Author

brendt commented Jan 14, 2020

@robertbasic I'm pretty sure it has nothing to do with Mockery.

@titanioverde
Copy link

I remember seeing this and other problems when writing my first tests, in a L5.1 application.
Until I changed in phpunit.xml the option processIsolation to true.
It's slower and patchy this way, but the memory usage kept from growing.

@tylermarien
Copy link

Is this issue similar to this one with Symfony: https://jolicode.com/blog/you-may-have-memory-leaking-from-php-7-and-symfony-tests? It is related to this issue in PHP: https://bugs.php.net/bug.php?id=76982. They mitigated it in Symfony, not sure if something similar can be used in Laravel.

@driesvints
Copy link
Member

I'm looking at this again and I've found the Symfony issue and workaround they implemented for them:

Issue: symfony/symfony#32220
Workaround: symfony/symfony#32236

Afaik, Symfony already released these and Laravel should run with these, including @mfn's test repo: https://github.com/mfn/laravel-tests-memleak

Does anyone has an idea how we could potentially work around this ourselves in Laravel? Otherwise I'm afraid there's not much we could do.

@driesvints
Copy link
Member

I'm going to close this one as there's really not much we can do here ourselves besides waiting for a fix on PHP's end. We're welcoming any suggestions on how to mitigate this in Laravel like Symfony did.

I'll pin this issue to the issue tracker for now so people can find it more easily.

@driesvints driesvints pinned this issue Apr 14, 2020
@taylorotwell taylorotwell unpinned this issue May 6, 2020
carlobeltrame added a commit to gloggi/qualix that referenced this issue Oct 1, 2020
According to laravel/framework#30736, there is
a confirmed memory leak in php (https://bugs.php.net/bug.php?id=76982)
that causes memory usage in PHPUnit tests to go up steadily. Everyone is
waiting for PHP to fix this, but in the meantime, we can still run our
tests by removing the PHP memory limit. Sooner or later, when adding
more tests, we might run into the container's memory limits though...
Hopefully the PHP bug will be fixed by then.
@willhatchVista
Copy link

For anyone who comes here after, I found an old bug report in the php framework that explained the root cause to be cycles and armed with this info I added a teardown to my base test class that catches these.
https://bugs.php.net/bug.php?id=75914
protected function tearDown(): void { parent::tearDown(); gc_collect_cycles(); }

@brendt
Copy link
Contributor Author

brendt commented Jul 23, 2021

I just want to leave a small update here for anyone still struggling with this. It appears that the underlying issue will be fixed (for the most part) in PHP 8.1:

In PHP 8.1, the memory leak for repeated included anonymous functions has been addressed by php/php-src#5595 (or at least, mostly addressed). The GC with destructors issue has been addressed by php/php-src#5581. I believe that covers the issues raised here.

@tylernathanreed
Copy link
Contributor

@brendt ,

I was able to find a workaround for this within a L7 / PHP 7.4 application. It didn't entirely fix the issue, but it did drastically reduce it. Our two main issues were coming from our route files and database factories. We started caching routes prior to tests, and modified the test suite to keep the base factory builder around between tests.

It's worth mentioning that class-based database factories (available in L8) should see a dramatic memory usage drop, as the registration and such under the hood is done is such a way that doesn't trip the memory leak in PHP.

Here's how we kept the database factory around between tests:

<?php

namespace Tests\Concerns;

use Illuminate\Contracts\Console\Kernel;
use Illuminate\Database\Eloquent\Factory as EloquentFactory;

trait CreatesApplication
{
    /**
     * The statically cached eloquent factory implementation.
     *
     * @var \Illuminate\Database\Eloquent\Factory
     */
    protected static $factory = null;

    /**
     * Creates the application.
     *
     * @return \Illuminate\Foundation\Application
     */
    public function createApplication()
    {
        $app = require __DIR__.'/../../bootstrap/app.php';

        $app->make(Kernel::class)->bootstrap();

        if (! is_null(static::$factory)) {
            $app->instance(EloquentFactory::class, static::$factory);
        }

        $this->beforeApplicationDestroyed(function () use ($app) {
            if (! is_null(static::$factory)) {
                return;
            }

            if ($app->bound(EloquentFactory::class)) {
                static::$factory = $app->make(EloquentFactory::class);
            }
        });

        return $app;
    }
}

This code waits for a test to need any model factory, and then once the base factory manager is bound to the application, it's reinjected so that it doesn't have to rebuild again.

Prior to these changes, we had maybe 50 MB leaking per 60 tests. After these changes, it dropped to ~1 MB leaking per 60 tests. Feature tests leaked a little more due to views, but wasn't more than ~5 MB per 60 tests.

It's not perfect, but it's much better than what we started with. It's given my team enough breathing room until we have the capacity to upgrade to PHP 8.1.

@bytestream
Copy link
Contributor

@tylernathanreed another solution is to integrate paratest for the majority of your test suite. Anything that doesn't play well with multiple processes can be ran in a separate test suite using phpunit. It's fairly easy to integrate Paratest without using Laravel's integration.

@tylernathanreed
Copy link
Contributor

@bytestream

We're actually using Paratest in our pipeline, and Paratest was failing for this reason. It's was unfortunately not a solution for us.

@brendt
Copy link
Contributor Author

brendt commented Dec 9, 2021

I just want to share an update, I ran my testsuite on PHP 8.1.0, and it still seems to leak memory. I guess it's a PHP issue more than anything else. I just wanted to leave this comment here for anyone doing research about this issue:

Screenshot 2021-12-09 at 13 16 20

Here's the related PHP bug report: https://bugs.php.net/bug.php?id=79519

@michaelklopf
Copy link

We ran into this issue too and a file with 52 tests takes 250 MB of memory. Running a test suite with 530 tests consumes 2.48 GB of memory. And all in all we have more than 2000 tests. PHP8.1 only did a small dent and decreased the amount of memory consumed to 230 MB.

The 250 MB tests just does assertions like this

        $model = MyClass::factory()->make();
        $model->state->transitionTo($state);
        $this->assertTrue($model->state->equals($state));

So all those objects are lying around and are not garbage collected if I understand this right, do I? Trying to unset the objects doesn't help to free the memory.

@olsavmic
Copy link

olsavmic commented Aug 8, 2022

We found an issue in php/php-src#5581 which caused a memory leak in PHP 8.1 in case destructors are present. Maybe it's related. A fix is ready in php/php-src#9265

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests