-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
PHPUnit tests almost 300% slower after Symfony update #24596
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
Comments
FWIW, the annotation loader should just disappear, assuming that |
@Ocramius - Do you think that the things you pointed out are the reason for the long durations and that they would be fixed in a v2 version, so that we "ignore" this issue until then? |
@Ocramius see my comment in doctrine/annotations#145 (comment) |
Maybe try https://github.com/dmaicher/doctrine-test-bundle. You can just use the "static cache" feature without the transactional connections. This reduces the time for my phpunit testsuite by factor 4-5. Also the memory consumption is less than half. |
I can try stuff out tomorrow since I'm not at work anymore. |
No, I don't know for sure - it needs to be profiled. |
@dmaicher - I installed the bundle and used this config:
This speeds up the tests by 1 minute, they now need 9 minutes and 52 seconds. |
@tzfrs ok interesting. For me the difference was more significant. I don't quite understand the difference between symfony 3.2 and 3.3 for you though. I also recently upgraded my app from 2.8 to 3.3 and for me the test suite performance actually improved slightly and did not become worse 😕 So why is it slower for you only on 3.3? |
That's what we're trying to figure out. There is already sth. I noticed. We currently have multiple bundles and when I run the tests in each bundle on it's own it doesn't matter if I'm on the 3.3 or 3.2 branch. The timings are always the same and together sum up to about ~ 6 minutes. But when I test the whole This is our phpunit config
Do you know if it's possible to list the duration per folder in PhpUnit after the tests are completed? That way I could check which folders take way longer. |
You could either try johnkary/phpunit-speedtrap to see which tests run slow, or use a profiler like XHProf or Blackfire |
For the records, I've experienced the same slowness when upgrading to Symfony 3.3 on a large application. |
@NicoHaase, thanks, I already wrote a custom ResultPrinter. We have some namespaces where tests are now like 1 or 2 minutes longer. I will investigate. These tests are integration/elastic search tests. Blackfire is also an option, we already have that integrated. If I don't find anything with the current approach I'll try Blackfire. |
Ok, I did some assertions and there are like 80 tests which take 1-2 seconds longer, some tests which take 5 seconds longer, some tests which take 10 seconds longer, so it seems like every test got slower. What would really help is to create a cachegrind file for only one method while testing all methods. Is it somehow possible? I only found ways to always profile EVERYTHING. |
If you just run a single test, you are able to profile that execution. But be sure to run the test with a warmed up cache |
Yes, but the problem is that running tests alone or just on a per bundle basis doesn't yield a different duration for the tests. The tests are only slower when I execute them all at once. |
Using XHProf, you could start running the profiler in the setup and write out the profile in the teardown of each test |
Just profile everything over two runs and compare? It will be a massive
cachegrind file, but it is ok-ish.
Marco Pivetta
http://twitter.com/Ocramius
http://ocramius.github.com/
…On Wed, Oct 18, 2017 at 12:04 PM, Theo Tzaferis ***@***.***> wrote:
Ok, I did some assertions and there are like 80 tests which take 1-2
seconds longer, some tests which take 5 seconds longer, some tests which
take 10 seconds longer, so it seems like every test got slower. What would
really help is to create a cachegrind file for only one method while
testing all methods. Is it somehow possible? I only found ways to always
profile EVERYTHING.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#24596 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakH_OGaFDf6gBu3xxmwced2R-3uJ4ks5stc0dgaJpZM4P8Xe3>
.
|
@tzfrs Do you use Symfony |
A lot of my tests are way slower due to about 8000 deprecated calls made which are getting services from the container in the tests. I understand this is now deprecated as those services may be inlined so they should be made public, but you don't care if it's only for testing purposes. Is there a way to disable those deprecations [regarding private services] in tests? |
@theofidry making those services public for the test env, that's the only way to do it. |
Just to be sure: you have disabled the profiler in your tests?
Interesting that in my test suite (which is huge and has tons of services retrieved directly from the container) I don't see any performance drop at all due to deprecations. |
Something to note: I usually run my tests with
I cannot easily benchmark using 3.2 currently. But maybe the difference between debug and no-debug is bigger on 3.3 now? |
@tzfrs did you have any luck finding out more? |
A Blackfire profile would immediately stop the guess work that's happening here, please provide one... |
Sorry! Totally forgot about that. @nicolas-grekas You're talking about a Blackfire profile for ALL tests? |
Or a subset of them that could hint the slowness yes. |
Would need to profile everything, because only doing a subset yields the same results. It's only slower when running ALL tests. I'll report back. |
Ah yes, some performance optimisations were indeed introduced. Still, can you try the replacement suggested in doctrine/annotations#162? |
@Ocramius Which replacement are we talking about? Or do you want me to simply do a |
@tzfrs find occurrences of |
@tzfrs you can try to run some of the tests with the blackfire CLI (https://blackfire.io/docs/cookbooks/profiling-cli) i.e.
With this, no need to modify the tests |
@Ocramius I replaced Result: So it got faster again! Thank you. However, as you said, this would mean patching Symfony manually. @iamluc Ah, I see! I'll try this and will report back. (Could however be, that I will report tomorrow, not today) |
Wow |
Sure, I am running it now with blackfire. Will update with results. Update: Sorry! I don't think I will make this in time, because of an appointment. But this will be the first thing I do tomorrow and I'll update you again. Thanks for your help guys. |
Good morning guys. I just ran the command again
Sadly, it seems like I can't profile my tests with blackfire/phpunit-bridge |
My diagnosis is that for some reason, your tests create many objects internally, and the PHP garbage collector is struggling with them (that's the hint provided by Blackfire) |
Yeah, we thought about that too. Especially since we have some legacy code where we access the container in UnitTests instead of mocking them (This is only a small part, however) and because our objects could have huge references (in the integration tests). So But yes, I think fixing deprecations should be quickest/best solution. If the tests are getting slower with each Symfony release I'm 99% sure that this is because of the depracations. At least this is the most logical explanation. I called Putting it into every test at the right point would be much thinking, because that way we could also call the I just added
to the Update: Setting |
Can you try something else?
if you change that 100 to a 10000, does it change anything? |
I did the tests (without the replacement of Result with 100: So it doesn't seem like it changed anything. |
…r (jrjohnson) This PR was merged into the 3.3 branch. Discussion ---------- When available use AnnotationRegistry::registerUniqueLoader | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | References tickets | #24596 | License | MIT Take advantage of AnnotationRegistry::registerUniqueLoader which will only add 'class_exists' as an autoloader if it has not already been added. This helps alleviate a performance issue when the same loader is added many times in tests. Commits ------- 97c3f42 Take advantage of AnnotationRegistry::registerUniqueLoader
I do not know if there is a connection with this issue, but maybe the doctrine/DoctrineBundle#763 issue can give an extra track, because I noticed an increase execution time of my functional tests caused by the double compilation of the cache for the container. |
Any update on this matter ? We upgraded from Symfony 2.8 to 3.4.7 and our PHPUnit functional test suite went from 10 min to 50 min. doctrine/annotations: v1.6.0 We will now try to use a profiler to see what's going on. |
If you did, could you share results please. |
This seems similar to #27053, only in my case tests blow up, but that can be nature of our tests, since we try to reuse variables there a lot. Are your containers being regenerated during the run? Meaning, is |
TLDR; Symfony introduced the Identify the problemAfter trying to isolate this test speed degradation, we came to the conclusion that only the After more debugging it turned out that our smoke test were solely checking whether certain GET-requests were asserted using SolutionAfter applying the following snippet, we were able to fix all tests and upgrade Symfony. The test suite even became a bit faster, because the redirects that did happen before are not in the test run anymore. # Symfony\Bundle\FrameworkBundle\Client
$client->setMaxRedirects(1);
$client->followRedirects(false); Hope this helps others to continue identifying where their tests fail. Proposed fixI'd propose to not use |
@rvanlaak the PR you linked does not introduces following redirects at all (this exists since ages). It only adds the fact that redirects are skipped for |
Will put some more time in a scenario to reproduce this asap. For now we're happy that we can roll out the Maybe referring to that PR was premature, but it did seem the most viable commit when I was looking at the commits that happened to |
@rvanlaak but is the issue related to tests using the client, or to tests booting a kernel (or to something else) ? the usage of the client might not be the only difference between these tests. |
My tests went after Upgrade from symfony 4.0.11 to 4.1.0 from
to
Around 350 unit tests, rest functional tests with code coverage tested. |
Same issue for the functional tests
Symfony 4.1.0
|
Without a profile, there is nothing to do here. I'm therefore closing. Don't hesitate to create new issues with actual information that could hint what is slow, e.g. with https://blackfire.io |
Our project was running
v3.2.6
and since it isn't supported anymore we wanted to switch to Symfony 3.3. We created a new branch based on our master branch and made some changes to the code (removing deprecated stuff, etc.) but no major changes.When running the tests of our project in the new branch it takes 15 minutes and 26 seconds whereas it only takes 6 minutes and 30 seconds in the master branch.
I found this issue: doctrine/annotations#135 (comment) and we updated our dependency to
doctrine/annotations
from 1.4.0 to 1.5.0.This already worked, it made our tests 5 minutes faster, so now it only takes 10 minutes and 50 seconds.
For all those tests I deleted the cache folders beforehand, did a
composer install
, etc. so I always have the same preconditions.In the linked PR from
doctrine/annotations
@Ocramius saidso is there nothing else that can be done to improve performance?
What we changed
kernel.root_dir
withkernel.project_dir
(same for the methodsgetRootDir()
inAppKernel
)trusted_proxies
fromconfig.yml
and now calling it in theapp.php
andapp_dev.php
But this alone shouldn't be responsible for an execution time that is 4 minutes longer, right?
The text was updated successfully, but these errors were encountered: