Skip to content

[FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap #21556

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
Feb 12, 2017

Conversation

nicolas-grekas
Copy link
Member

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

Related to #21381 which disabled any cache at bootstrap.
Instead, this wires ArrayCache during bootstrapping, then swaps the cache provider for the real one.

if ($container->hasAlias($provider = $tags['annotations.cached_reader'][0]['provider'])) {
$provider = (string) $container->getAlias($provider);
}
$container->set('annotations.cached_reader', null);
Copy link
Member

Choose a reason for hiding this comment

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

aren't we deprecating this ?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Feb 7, 2017

Choose a reason for hiding this comment

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

not in #21533 (methodMap is empty in ContainerBuilder), but in #19668 yes - yet I think we should not merge it as explained implicitly :)

@tristanbes
Copy link
Contributor

tristanbes commented Feb 8, 2017

This patch seems to fix this issue: #21559 that is present on v3.2.3

As you can see on blackfire:
v3.2.2 to v3.2.3 patched
and
v3.2.3 not patched to v3.2.3 patched

This patch still have a +2,39% negative impact on performance, but it's still better than +2020% of negative impact :-)

@lubomir-haralampiev
Copy link

also fixes the #21576

Thanks

@fabpot
Copy link
Member

fabpot commented Feb 12, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit f90f53e into symfony:3.2 Feb 12, 2017
fabpot added a commit that referenced this pull request Feb 12, 2017
… bootstrap (nicolas-grekas)

This PR was merged into the 3.2 branch.

Discussion
----------

[FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap

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

Related to #21381 which disabled any cache at bootstrap.
Instead, this wires ArrayCache during bootstrapping, then swaps the cache provider for the real one.

Commits
-------

f90f53e [FrameworkBundle] Wire ArrayCache for annotation reader at bootstrap
@nicolas-grekas nicolas-grekas deleted the fwb-boot-annot branch February 16, 2017 22:45
@fabpot fabpot mentioned this pull request Feb 17, 2017
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