8000 Container building performance with lazy autowire error messages · Issue #32711 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Container building performance with lazy autowire error messages #32711

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.

Sign up for GitHub

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

Jump to bottom
Closed
bsod85 opened this issue Jul 24, 2019 · 7 comments
Closed

Container building performance with lazy autowire error messages #32711

bsod85 opened this issue Jul 24, 2019 · 7 comments
Labels
DependencyInjection Performance

Comments

@bsod85
Copy link
bsod85 commented Jul 24, 2019

Symfony version(s) affected: 4.3.*

Description
Performance of container building has been degraded after 4.3 update

How to reproduce
https://github.com/bsod85/sf_autowire_performance
By switching the symfony/dependency-injection between 4.2.* and 4.3.*, after a composer update I obtain these results with the command "time bin/console cache:clear":
Version 4.3 -> 107,57s user 0,49s system 99% cpu 1:48,07 total
Version 4.2 -> 24,77s user 0,45s system 99% cpu 25,233 total

Additional context
After a bisect I came to the conclusion that the problem has been introduced in commit 3b3a1bd
The AutowirePass::createTypeNotFouldMessageCallback() method creates a new copy of the entire container for each encountered error.
This severely hinders the performance on big projects if many non-autowirable classes are processed by the "resource" option.
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jul 24, 2019 •
edited
Loading

Can you please provide an https://blackfire.io comparison before/after the commit you mention?

@nicolas-grekas nicolas-grekas mentioned this issue Jul 24, 2019
Merged
@nicolas-grekas
8000
Copy link
Member

Can you please also check if #32715 improves the situation?

@bsod85
Copy link
Author
bsod85 commented Jul 25, 2019

Had to apply this fix to make the specific commit work
b092502

Here are the profiles:
before: https://blackfire.io/profiles/232b3e72-f8cf-4438-b8b7-798bc6004e83/graph
after: https://blackfire.io/profiles/8c90e987-3e20-4762-9f6d-ed9f4ac2b959/graph
with fix: https://blackfire.io/profiles/3c0b255e-7bdd-499b-8013-e99f46751592/graph

The fix seems to improve the situation but still much time is spent in the setDefinitions method.
Shouldn't the lazy message callback be called only when the error is to be displayed? Looking at the profile it doesn't seem so.

Let me know if there's anything else I can do

@nicolas-grekas
Copy link
Member

Thanks for the profile, that's really eliminating the guesswork!
Can you try the updated patch in the PR and post the resulting profile, please?

@bsod85
Copy link
Author
bsod85 commented Jul 26, 2019

https://blackfire.io/profiles/76a5be15-be03-4f5b-9d38-af355f647355/graph

Now it seems much better

@nicolas-grekas
Copy link
Member

Thanks, problem solved then. PHP 7.3 will provide an additional perf improvement by reducing the pressure on the GC I think.

@bsod85
Copy link
Author
bsod85 commented Jul 26, 2019

Thank you very much

Tobion added a commit that referenced this issue Jul 26, 2019
@Tobion
bug #32715 [DI] fix perf issue with lazy autowire error messages (nic…
61b0f79 
…olas-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[DI] fix perf issue with lazy autowire error messages

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32711
| License       | MIT
| Doc PR        | -

See linked issue.

Commits
-------

3c3bda5 [DI] fix perf issue with lazy autowire error messages
@xabbuh xabbuh closed this as completed Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection Performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @xabbuh @bsod85
0