-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
Possible memory leak #44214
Comments
Adding this line: \Illuminate\Console\Application::forgetBootstrappers(); Right after And commenting the last 2 bootstrapers in: framework/src/Illuminate/Foundation/Console/Kernel.php Lines 82 to 90 in f40fe7d
Further reduced from 28MB to 18MB. (in my machine it starts with 12MB) This last one is less objective (could be any loaded service provider), and impacts the application. But at least it gives a hint on where to look further. |
I've found a slightly better way to illustrate that there is no code outside of Laravel causing the memory usage to increase. Add the following test class as class ExampleTest extends TestCase
{
protected function tearDown(): void
{
parent::tearDown();
dump(sprintf('%d MB', (int) (memory_get_usage(true) / (1024 * 1024))));
}
public function test_true()
{
$this->assertTrue(true);
}
} Run the test repeatedly using the
|
Lol that's my article. I assume these test are done in vanilla Laravel. Did anyone tried the Opcache trick? BTW, seems that |
Seems this is indeed still a culprit even after a fix in PHP core was merged here: php/php-src#9265. Also see #30736 @olsavmic do you might have knowledge of any other memory leaks in PHP core?
@DarkGhostHunter I disabled that call and the memory leak still happens. Also, I think for this to be investigated we shouldn't focus on Opcache? Not sure what that has to do with anything. |
I'd like to continue digging into this but not sure how to proceed (aside from a lot of code commenting). Is there an easy way to profile the code while running tests? |
@driesvints I know but is just a workaround while we pinpoint the culprit. |
@PHLAK not that I know. Just use |
Maybe xdebug profiler could be helpful in this connection. |
I've profiled my application code while running tests. This generated several |
Just pinging you @brendt as well (since you originally opened the other issue) to see if you had any thoughts here? |
Going to take a look at this next Monday. ✅ |
Has anyone tried to recreate the issue on initial release of Laravel 8.0? or Laravel 7.0? So we can see possibly when the memory leak was introduced if it is indeed something new. Or, if it something that has been around for some time. |
I ran the example test against Laravel v8.83.24 with similar results. I'm getting errors when trying to install Laravel 7 (i.e. |
There is a discussion about this as well - [8.x] Memory leak on testing #39255. Might have some useful info |
I found and issue with registration of error handlers in Illuminate\Foundation\Bootstrap\HandleException. I tried to fix it in #44293 but it's not trivial (as already stated in the original MR #40656 (comment)). It just the one class that's leaking and the memory increases very slowly so I guess it's not the problem the author of this issue is dealing with :/ @PHLAK just for reference, the original test from the article you used is not performing the cleanup completely which is the reason it leaks so fast. Look at \Illuminate\Foundation\Testing\TestCase::tearDown, line 220 - 222. The second test case with repeated PHPUnit test case runs works correctly but it leaks significantly slower (and stops leaking after the fix is applied). |
Friends, can you please try out your test suite on the latest |
I’ll do it after I get out of work today. Italo Baeza C.El 26-09-2022, a la(s) 15:04, Nuno Maduro ***@***.***> escribió:
Friends, can you please try out your test suite on the latest 9.x-dev branch, and tell me the before / after result?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I ran my app's test suite locally with and without the applied fixes and oddly the memory consumed was identical between the runs EDIT: I re-ran the tests multiple times and the time was more consistent between runs. The difference in reported time was probably a fluke. As for the memory usage being identical across runs I think this has to do with parallel testing. Before Fix
EDIT: Additional runs
After Fix
I then pushed a branch with the fixes to CI and it failed the same as before. All of these tests were run with the |
Also seeing no improvement (non-parallel test run)... Before
After
|
I did a few more (partial) test runs with before and after fix code, this time not using the In both cases the memory usage continued to grow indefinitely. However, with the fixes applied the memory growth appeared (subjectively) to grow much slower and was (objectively) much lower in the end (625 MB before fixes vs 252 MB after fixes). I also noticed a few points where the memory reported went down (albeit only one or two MB) between reportings with the fixed code. Here are the results. Before Fix
After Fix
|
One more thing to note, I just installed a fresh Laravel app from the
|
I wrote this shell script (tested in Linux) that:
#!/bin/sh
########### options and variables
while getopts 'u' options; do
case $options in
u) namespace="Tests\Unit";;
esac
done
shift $((OPTIND-1))
project=$1
namespace=${namespace:-"Tests\Feature"}
parent=$([ $namespace == "Tests\Feature" ] && echo "Tests\TestCase" || echo "PHPUnit\Framework\TestCase")
########### script commands
rm -rf $project
composer create-project laravel/laravel $project
cd $project || exit
composer require laravel/framework:9.x-dev
# verify git hash
composer show laravel/framework | grep "source :"
# verify nuno's last commit
sed -n -e '220,221p' ./vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php
# give a chance to verify
sleep 3
rm ./tests/Unit/ExampleTest.php
rm ./tests/Feature/ExampleTest.php
i=1
while [ $i -le 1000 ]; do
prefix=$(printf 'Case%04d' $i)
echo "<?php
namespace ${namespace};
use ${parent};
class ${prefix}Test extends TestCase
{
public function test_memory_leak()
{
echo \PHP_EOL, 'Memory: ', ((int) (memory_get_usage(true) / (1024 * 1024))), 'MB';
\$this->assertTrue(true);
}
}" > ./tests/Feature/${prefix}Test.php
i=$(( $i + 1 ))
done
php artisan test
cd ..
rm -rf $project It accepts a It also requires a project name. Note that the project folder will be forcefully removed before and after execution. Below are the memory outputs, truncated to just the first and last test output for brevity Running for Feature Tests$ ./create-test.sh leak-test
Memory: 56MB
PASS Tests\Feature\Case0001Test
✓ memory leak
# ...
Memory: 238MB
PASS Tests\Feature\Case1000Test
✓ memory leak
Tests: 1000 passed
Time: 4.22s Running for Unit Tests$ ./create-test.sh -u leak-test
Memory: 38MB
PASS Tests\Unit\Case0001Test
✓ memory leak
# ...
Memory: 42MB
PASS Tests\Unit\Case1000Test
✓ memory leak
Tests: 1000 passed
Time: 0.15s Note that there is a small overhead when running only Unit Tests. This is also present when running the tests with the Hope this helps. |
Thanks @rodrigopedra but it's unecessary to create 1000 actual test cases. You can use |
Got similar results. Does this happen with other testing frameworks? |
@PHLAK thanks for reviewing it. When running a single test class with
That is why I created 1,000 test cases, to mimic an environment with several test classes instead of repeating a single test over and over. You can verify this with the slightly modified script below. Changed lines have a comment Usage is the same as before: #!/bin/sh
########### options and variables
while getopts 'u' options; do
case $options in
u) namespace="Tests\Unit";;
esac
done
shift $((OPTIND-1))
project=$1
namespace=${namespace:-"Tests\Feature"}
parent=$([ $namespace == "Tests\Feature" ] && echo "Tests\TestCase" || echo "PHPUnit\Framework\TestCase")
########### script commands
rm -rf $project
composer create-project laravel/laravel $project
cd $project || exit
composer require laravel/framework:9.x-dev
# verify git hash
composer show laravel/framework | grep "source :"
# verify nuno's last commit
sed -n -e '220,221p' ./vendor/laravel/framework/src/Illuminate/Foundation/Testing/TestCase.php
# give a chance to verify
sleep 3
rm ./tests/Unit/ExampleTest.php
rm ./tests/Feature/ExampleTest.php
i=1
while [ $i -le 1 ]; do # CHANGED
prefix=$(printf 'Case%04d' $i)
echo "<?php
namespace ${namespace};
use ${parent};
class ${prefix}Test extends TestCase
{
public function test_memory_leak()
{
echo \PHP_EOL, 'Memory: ', ((int) (memory_get_usage(true) / (1024 * 1024))), 'MB';
\$this->assertTrue(true);
}
}" > ./tests/Feature/${prefix}Test.php
i=$(( $i + 1 ))
done
php artisan test --repeat=1000 # CHANGED
cd ..
rm -rf $project |
Thank you for sharing your feedback. To keep the output of everyone focused, please only share results of real test suites without the parallel flag (not repeats, not scripts, not loops), and share the output of PHPUnit via
|
@nunomaduro , thanks for looking into it. The app I tested only has the standard breeze tests installed. It is a real app, but without any further tests written. The only change I made was just adding the memory output on the <?php
namespace Tests;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
protected function tearDown(): void
{
parent::tearDown();
echo \PHP_EOL, 'Memory: ', ((int) (memory_get_usage(true) / (1024 * 1024))), 'MB';
}
} If you have a better way of tracking memory usage, let me know. Running Laravel
|
Going to keep working on this. For info everyone, there is a memory leak on the package bugsnag. |
Are you all using Bugsnag? |
For info everyone, here is the recap of my investigation yesterday. And going to keep working on it today. Use case: Using Vapor's test suite, more than 600 HTTP / Jobs / Console tests.
|
@driesvints I am not |
So, to keep everyone on the same page, it's important to you'll read this. In what concerns memory leaks on the framework, in this issue, and previous issues regarding this topic, there are two types of reports been done by the community so far: A. The people say that running 100,000 tests causes the memory to go from 22MB to 70MB - and keeps growing. Often, these reports are explained using things like The case A is not relevant and it's caused by two things: the peak of memory of PHP (including files and classes) and related to #40656, as Laravel sets a new On this issue, it's only important reports of the case B, again, real-world test suites, with hundreds of tests, where there is a real memory leak affecting the test suite, and that memory leak causes the memory to go from 22MB to hundreds of MB (or even memory exhausted). Hope this helps to keep the discussion focused. |
@nunomaduro noted. The project I work on with the most exhaustive amount tests is still on Laravel 8 (production server stuck to PHP 7.4 due to some other reasons) The project I ran, with the breeze suite, was a recent pro-bono small project I made for a charity. So thanks all for the efforts, but I will not be able to help you out with this anymore. Good luck on tackling this down ! |
@driesvints No, we're not using Bugsnag.
I will try this soon. |
@nunomaduro To do an apples-to-apples comparison to my previous run I used I then ran it using
This downward trend is good to see. 📉 👍 |
So we can start gathering before/after results, here is how you can help if you have a big test suite:
So, the results so far, since the very beginning, comparing
|
As discussed with @taylorotwell, we are happy with the results so far. So closing this issue. |
For the record, here's the results for my full test suite.
Very impressive results indeed! Thanks for all the work @nunomaduro and others! |
I know I'm a little late to the party, but I've re-created the tests of @PHLAK on a clean installation (the --repeat option has been removed from PHPUnit it seems?, so I've just duplicated a lot of tests instead). After some back and forth, I've found the option of "processIsolation" in PHPUnit, which when I set that to true, seems to solve the memory leak issue for me. All I've added was to the tearDown of TestCase to see memory usage. processIsolation = false processIsolation = true The testing time on this clean install increased dramatically, while on another project (4700 tests) when running in parallel, it runs for about the same time, but without using 20ish GB of ram. Though I'd just share my two cents. I don't know if there are any disadvantages to changing the processIsolation, maybe someone else has an idea? **** Update **** |
Description:
We've been having memory related issues in CI recently where the memory consumption continuously grows. This is a particular problem when enabling coverage checks (i.e.
artisan test --coverage
) as this eventually exhausts the memory in the CI environment. A few weeks ago we thought we had this traced back to this issue in PHP core which was fixed as of v8.1.10, however, after updating to 8.1.10 we're still having memory issues in CI.I recently found this article which recommends creating a test to repeatedly create, boot and flush the (Laravel) application multiple times to see the memory consumption over time. I did this on a fresh Laravel application and, as you can see below, the memory usage constantly rises over time.
I tried to reproduce this on a personal project that doesn't use Laravel to determine if it was an issue with phpunit or PHP core but was unable to reproduce the leak so I'm leaning towards Laravel at the moment (however, I'm not convinced of this yet).
I haven't dug any deeper at this point as I wanted to bring this to your attention but am happy to keep investigating if necessary.
Steps To Reproduce:
Create a new Laravel application.
Add the following test case to the newly created application.
Run the test and observe the ever increasing memory consumption.
The more iterations, the more memory used.
The text was updated successfully, but these errors were encountered: