Skip to content

Lazy load Translator service or make constructor lighter #27010

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
grisendo opened this issue Apr 22, 2018 · 6 comments
Closed

Lazy load Translator service or make constructor lighter #27010

grisendo opened this issue Apr 22, 2018 · 6 comments
Labels
FrameworkBundle Performance Translation

Comments

@grisendo
Copy link

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 3.x (maybe 2.x and 4.x also?)

I am debugging a custom CMS made with Symfony (3.x) using Blackfire, in order to improve the performance as much as possible (and learn!). I already removed all heavy constructor services (Doctrine, Twig/Templating) from event listeners/subscribers, but there is one which I can't (or maybe shouldn't) remove, since it's in core. I am talking about Symfony\Component\HttpKernel\EventListener\TranslatorListener, which has "translator" service as dependency, and that service has a heavy constructor. It takes ~10% of the time in my current best case scenario.

I was looking for some issue similar to this, to no create a duplicate, I found some others about improve it (i.e. limiting some loaders/dumpers), but I didn't find any issue about lazy-loading that service (or its heavy logic) to avoid its construction at all unless required.
@Tobion
Copy link
Contributor

Tobion commented Apr 22, 2018

Making the translator lazy would not change anything. The event listener sets the locale on the translator. This initializes the translator even if it was lazy.

So instead you need to make the translator constructor itself faster, e.g. by lazy loading one of the dependencies.

@grisendo
Copy link
Author

You're right, I agree about making the constructor faster. I didn't take a detailed look yet, but it looks like '$this->addResourceFiles' inside the constructor can be the bottleneck here. Maybe it would be better to not do it there, and load them when really trying to get them (checking load status by checking if $this->resources is empty, or with another $resourceFilesLoaded property or something).

@xabbuh
Copy link
Member

xabbuh commented Apr 23, 2018

Could you make the profile public and add a reference to it here? :)

@grisendo
Copy link
Author

https://blackfire.io/profiles/8e320963-fced-48b8-a7ca-d2f55426f772/graph

@chalasr
Copy link
Member

chalasr commented Apr 24, 2018 •
edited
Loading

The Translator::$resources private prop is all about lazy loading:

symfony/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php

Lines 40 to 46 in e775871

/**
* Holds parameters from addResource() calls so we can defer the actual
* parent::addResource() calls until initialize() is executed.
*
* @var array
*/
private $resources = array();

and the fact it is set from the constructor has been challenged in #23057 (comment) but apparently not addressed.

No blocker for moving the addResourceFiles() call to the initialize() method to me 👍

@nicolas-grekas nicolas-grekas mentioned this issue Apr 26, 2018
Merged
@nicolas-grekas
Copy link
Member

See #27063

nicolas-grekas added a commit that referenced this issue Apr 29, 2018
@nicolas-grekas
minor #27063 [Translator] Further postpone adding resource files (nic…
d843181 
…olas-grekas)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Translator] Further postpone adding resource files

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

Commits
-------

2aa62df [Translator] Further postpone adding resource files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FrameworkBundle Performance Translation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @Tobion @grisendo @xabbuh @chalasr