Skip to content

[DI] skip preloading dependencies of non-preloaded services #36535

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

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

Suggested by @stof on Slack: this improves preloading by propagating the container.no_preload tag to services that are referenced only by not-preloaded services.

The benefit is double:

  1. this fixes potential over-preloading
  2. this requires less work from the community: no need to add the tag anymore most of the time

As a corollary, listeners of console events are tagged with container.no_preload automatically now.

}
}
if ($noPreload && \count($extractingDispatcher->listeners) === $noPreload) {
$container->getDefinition($id)->addTag($this->noPreloadTagName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a class is both a subscriber and tagged with additional kernel.listener tags ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we don't really care IMHO: it should be exceptional, thus not worth any extra complexity. And the outcome is minor anyway.

@fabpot
Copy link
Member

fabpot commented Apr 24, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit ac3bd14 into symfony:master Apr 24, 2020
@nicolas-grekas nicolas-grekas deleted the di-no-preload branch April 24, 2020 10:19
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
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.

6 participants