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

Fix all memory leaks #1672

Merged
merged 6 commits into from
Oct 7, 2021
Merged

Fix all memory leaks #1672

merged 6 commits into from
Oct 7, 2021

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Oct 5, 2021

fixes #1669

All tests must be able to run 1 000 000 times without any memory increase 😄

The following usecases are leaky by design:

minimal leak per iteration, leaks when the array is resized (4x, 8x, 16x, ...)

https://3v4l.org/JAMjY

if (!class_exists(Leaky::class)) {
    class Leaky
    {
        public static $arr = [];
    }
}
Leaky::$arr[] = count(Leaky::$arr);

leaks when php resizes class table (also in multiple of 2, but as many classes are normally already present, takes much more iterations to detect)

https://3v4l.org/jrqso (demo should include the code below in a look and the results depends if opcache is enabled)

new class() {};

@mvorisek mvorisek force-pushed the no_mem_leaks branch 8 times, most recently from 2720a30 to 9aba083 Compare October 5, 2021 23:47
@mvorisek mvorisek mentioned this pull request Oct 5, 2021
@mvorisek mvorisek force-pushed the no_mem_leaks branch 8 times, most recently from d398b10 to 7061b91 Compare October 6, 2021 16:31
@mvorisek mvorisek force-pushed the no_mem_leaks branch 2 times, most recently from b96303d to 607bf72 Compare October 6, 2021 16:52
@mvorisek mvorisek added the RTM label Oct 6, 2021
@mvorisek mvorisek marked this pull request as ready for review October 6, 2021 17:24
@mvorisek
Copy link
Member Author

mvorisek commented Oct 6, 2021

I verified 1600 iterations of unit tests:

PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.0RC3
Configuration: /__w/ui/ui/phpunit.xml.dist

...............................................................  63 / 497 ( 12%)
............................................................... 126 / 497 ( 25%)
.........................F..................................... 189 / 497 ( 38%)
............................................................... 252 / 497 ( 50%)
............................................................... 315 / 497 ( 63%)
............................................................... 378 / 497 ( 76%)
............................................................... 441 / 497 ( 88%)
........................................................        497 / 497 (100%)

You should really speed up these slow tests (>500ms)...
 1. 180392ms to run Atk4\Ui\Tests\JsTest:testNumbers
 2. 46438ms to run Atk4\Ui\Tests\DemosTest:testDemosStatusAndHtmlResponse with data set #59
 3. 46042ms to run Atk4\Ui\Tests\DemosTest:testDemosStatusAndHtmlResponse with data set #49
 4. 41555ms to run Atk4\Ui\Tests\DemosHttpNoExitTest:testDemosStatusAndHtmlResponse with data set #49
 5. 40208ms to run Atk4\Ui\Tests\DemosHttpTest:testDemosStatusAndHtmlResponse with data set #49
 6. 40152ms to run Atk4\Ui\Tests\DemosHttpNoExitTest:testDemosStatusAndHtmlResponse with data set #59
 7. 39672ms to run Atk4\Ui\Tests\DemosHttpTest:testDemosStatusAndHtmlResponse with data set #59
 8. 34168ms to run Atk4\Ui\Tests\DemosHttpNoExitTest:testDemosStatusAndHtmlResponse with data set #28
 9. 32179ms to run Atk4\Ui\Tests\DemosHttpTest:testDemosStatusAndHtmlResponse with data set #80
 10. 31640ms to run Atk4\Ui\Tests\DemosHttpTest:testDemosStatusAndHtmlResponse with data set #105
...and there are 404 more above your threshold hidden from view

Time: 01:25:12.599, Memory: 46.00 MB

There was 1 failure:

1) Atk4\Ui\Tests\DemosTest::testDemosStatusAndHtmlResponse with data set #19 ('basic/button.php')
Memory leak detected! (128.031 + 0.625 + 0.313 KB, 17 iterations)

FAILURES!
Tests: 497, Assertions: 2117919, Failures: 1.

the one failure is leaky experiment correctly detected.

@mvorisek mvorisek merged commit bf817f9 into develop Oct 7, 2021
@mvorisek mvorisek deleted the no_mem_leaks branch October 7, 2021 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Something leaks memory
2 participants