[Routing] allow using compiled matchers and generators without dumping PHP code #28865

nicolas-grekas · 2018-10-14T17:40:44Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#29590
License	MIT
Doc PR	symfony/symfony-docs#10790

This is a resurrection of #25909 to make matcher+generator dumpers output PHP arrays instead of PHP code.
Don't be fooled by the diff stats, it's mostly fixtures.

This PR should contribute to making the Routing component easier to use standalone.

On the way back from SFLive USA.

src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php

nicolas-grekas · 2018-10-16T15:52:17Z

Ready, failure unrelated. As is, this PR will enable the much faster CompiledUrlMatcher/Generator by default, even when using a non-dumped one.

src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableCompiledUrlMatcher.php

src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php

javiereguiluz

Thanks for this! In order to be mergeable, could you please update the PR description to add a simple example that uses this feature? I guess this is only for the stand-alone component, not full Symfony apps. Thanks!

Note: I'm "blocking" this PR with a "Request Changes" review as suggested by the Symfony community on Symfony Slack. Some people suggested to "block" features until they are documented ... so I'm trying that to see how it goes. Thanks!

nicolas-grekas · 2019-01-22T22:01:13Z

Without more feedback, I'm going to merge this PR this week.

fabpot · 2019-01-23T13:04:56Z

Can you rebase please?

…g PHP code

nicolas-grekas · 2019-01-23T15:25:01Z

rebased!

… without dumping PHP code (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [Routing] allow using compiled matchers and generators without dumping PHP code | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #29590 | License | MIT | Doc PR | symfony/symfony-docs#10790 This is a resurrection of #25909 to make matcher+generator dumpers output PHP arrays instead of PHP code. Don't be fooled by the diff stats, it's mostly fixtures. This PR should contribute to making the Routing component easier to use standalone. On the way back from SFLive USA. ![image](https://user-images.githubusercontent.com/243674/46920076-784e1b80-cf9d-11e8-86e7-850fffb409de.png) Commits ------- f0a519a [Routing] allow using compiled matchers and generators without dumping PHP code

… (nicolas-grekas) This PR was merged into the master branch. Discussion ---------- Doc CompiledUrlMatcherDumper instead of PhpMatcherDumper Needed after symfony/symfony#28865 Commits ------- 8e8df56 Doc CompiledUrlMatcherDumper instead of PhpMatcherDumper

Tobion · 2019-02-14T03:39:51Z

Late review:: Looks fine to me. Being able to achieve the same thing by only dumping the data structure instead of the executing code makes it a little cleaner and more flexible.

Fix small typos from #28865

Tobion · 2019-02-14T04:05:42Z

One thing though: We should deprecate generator_base_class, generator_cache_class, matcher_base_class and matcher_cache_class config options. Those options are useless now and unused with the compiled variants. And there is a potential BC break currently: If someone overwrote the base_class config without changing the generator_class/matcher_class, it won't work anymore because it will use the compiled class which ignores the base_class. So we should add a check if the base_class was changed before using the compiled versions.

Tobion · 2019-02-14T04:06:46Z

I might be able to fix this tomorrow.

This PR was merged into the 4.3-dev branch. Discussion ---------- [Routing] fix changelog typo | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | | License | MIT | Doc PR | Fix small typos from #28865 Commits ------- 37dbab2 [Routing] fix changelog typos

This PR was merged into the 4.3-dev branch. Discussion ---------- [Routing] deprecate some router options | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no  | BC breaks? | no  | Deprecations? | yes  | Tests pass? | yes  | Fixed tickets | symfony/symfony#28865 (comment) | License | MIT | Doc PR | symfony/symfony-docs#...   Commits ------- bf4cd6164d [Routing] deprecate some router options

This PR was merged into the 4.3-dev branch. Discussion ---------- [Routing] deprecate some router options | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no  | BC breaks? | no  | Deprecations? | yes  | Tests pass? | yes  | Fixed tickets | #28865 (comment) | License | MIT | Doc PR | symfony/symfony-docs#...   Commits ------- bf4cd61 [Routing] deprecate some router options

…Fayet) This PR was merged into the 4.3 branch. Discussion ---------- [Router] routing cache crash when using generator_class | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31807 | License | MIT Since #28865 the Router use, by default, new generator, matcher, and dumpers. This leads to crash when the Router use a custom generator, or matcher based on the old ones. Commits ------- a5b46e5 Fix routing cache broken when using generator_class

This PR was merged into the 4.3 branch. Discussion ---------- Fix version typo in deprecation notice | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | N/A #28865 was merged into 4.3 Commits ------- bd13271 Fix version typo in deprecation notice

…eMC) This PR was merged into the 4.3 branch. Discussion ---------- [Routing] Fix URL generator instantiation | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT After merging my fix (PR #31023) from the 4.2 branch to 4.3 (which was master at the time) one code path which instantiates the url generator was missed. This new code path was added via #28865 and was missed during the merge because there was no merge conflict there as that code path didn't exist on 4.2. So the current state on the 4.3 branch is: - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L356 (default locale is properly passed to the constructor) - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L372 (default locale is **NOT** properly passed to the constructor) - https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L378 (default locale is properly passed to the constructor) I noticed this in my app because tests started to fail on the branch where we are upgrading our app from SF 4.2 to 4.3 due to this. This PR fixes the inconsistency (now the default locale is passed on all three places) which fixes the bug. Commits ------- 9aa66e2 Add tests to ensure defaultLocale is properly passed to the URL generator 16c9baf Fix URL generator instantiation

…ault) This PR was squashed before being merged into the 5.2-dev branch. Discussion ---------- [Serializer] Add CompiledClassMetadataFactory | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ? | License | MIT | Doc PR | todo (issue: symfony/symfony-docs#10706) This introduce a dumped `ClassMetadataFactoryInterface` implementation to speed up the serializer by leveraging [PHP7 immutable array](https://blog.blackfire.io/php-7-performance-improvements-immutable-arrays.html). Like for #28865, if the user have the opcache extension enabled, the compilation time will be skipped. The user will also have a performance boost when not using opcache as we are no longer fetching `ClassMetadata` from the PSR-6 cache. This allow to speed up the normalization (without opcache) by 9-12% depending on how many objects are involved in the graph: - [SymfonyObjectNormalizerBenchmark, 100 iterations with complexity of 1](https://blackfire.io/profiles/compare/d937a9cc-eebf-47eb-be90-c8e65cdf12b3/graph) - [SymfonyObjectNormalizerBenchmark, 3 iterations with complexity of 60](https://blackfire.io/profiles/compare/d490542c-9a79-48a0-b7bc-1ed3ca6a9148/graph) On the `FrameworkBundle` side, I suggest to add a `CacheWarmer` to dump the metadata array from configured class list. The list could have a _good_ default which will load the classes found in `src/Entity`. ## Dumping the `ClassMetadata` ```php $classMetadatas = []; foreach([Category::class, Comment::class, Forum::class, Thread::class, User::class] as $class) { $classMetadatas[] = $this->classMetadataFactory->getMetadataFor($class); } file_put_contents('dumped.php', $this->classMetadataFactoryCompiler->compile($classMetadatas)); ``` ## Using the dumped `ClassMetadata` ```php $classMetadataFactory = new CompiledClassMetadataFactory( 'dumped.php', new CacheClassMetadataFactory( new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader())), new ApcuAdapter('SymfonyMetadata') ) ); ``` # To do - [x] Tests. - [x] Cache warmer. - [x] Documentation. - [x] Changelog entry. Commits ------- 63cbf0a [Serializer] Add CompiledClassMetadataFactory

nicolas-grekas added this to the next milestone Oct 14, 2018

carsonbot added Status: Needs Review Routing Feature Deprecation labels Oct 14, 2018

nicolas-grekas mentioned this pull request Oct 14, 2018

[Routing] allow using the compiled URL generator without dumping PHP code #28774

Closed

nicolas-grekas force-pushed the route-static branch 2 times, most recently from 1012b2a to 1d5e088 Compare October 14, 2018 21:11

Shine-neko reviewed Oct 15, 2018

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php Show resolved Hide resolved

nicolas-grekas force-pushed the route-static branch 4 times, most recently from f26ec0e to 51cdea8 Compare October 16, 2018 01:08

stof reviewed Oct 16, 2018

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableCompiledUrlMatcher.php Show resolved Hide resolved

src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the route-static branch from 51cdea8 to 46819ec Compare October 17, 2018 07:07

stof approved these changes Oct 17, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 17, 2018

This was referenced Nov 6, 2018

[Serializer] Add a NormalizerDumper #22051

Closed

[Serializer] Add CompiledClassMetadataFactory #29117

Merged

javiereguiluz requested changes Nov 7, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Nov 7, 2018

nicolas-grekas mentioned this pull request Nov 30, 2018

[Router] Remove goto in RedirectableUrlMatcher #29362

Closed

nicolas-grekas force-pushed the route-static branch 2 times, most recently from 0e530fd to 8ed58fa Compare December 2, 2018 13:28

nicolas-grekas mentioned this pull request Dec 14, 2018

[Router][Feature] Append new router to cache container without regenerating it. #29590

Closed

nicolas-grekas force-pushed the route-static branch from 8ed58fa to 80c6d28 Compare December 17, 2018 15:58

nicolas-grekas mentioned this pull request Dec 17, 2018

Doc CompiledUrlMatcherDumper instead of PhpMatcherDumper symfony/symfony-docs#10790

Merged

[Routing] allow using compiled matchers and generators without dumpin…

f0a519a

…g PHP code

nicolas-grekas force-pushed the route-static branch from d5be794 to f0a519a Compare January 23, 2019 15:21

nicolas-grekas merged commit f0a519a into symfony:master Jan 26, 2019

nicolas-grekas deleted the route-static branch January 26, 2019 20:53

Tobion added a commit that referenced this pull request Feb 14, 2019

[Routing] fix changelog typos

37dbab2

Fix small typos from #28865

Tobion mentioned this pull request Feb 14, 2019

[Routing] fix changelog typo #30234

Merged

Tobion mentioned this pull request Feb 14, 2019

[Routing] deprecate some router options #30249

Merged

nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

fabpot mentioned this pull request May 9, 2019

Release v4.3.0-BETA1 #31435

Merged

dFayet mentioned this pull request Jun 9, 2019

[Router] routing cache crash when using generator_class #31964

Merged

teohhanhui mentioned this pull request Sep 23, 2019

Fix version typo in deprecation notice #33668

Merged

X-Coder264 mentioned this pull request Oct 30, 2019

[Routing] Fix URL generator instantiation #34192

Merged

apetitpa mentioned this pull request Nov 30, 2019

[Routing] Generate URL for internationalized routes with the _locale given as parameter getting removed #34469

Closed

FlyingDR mentioned this pull request Nov 6, 2020

missing route Haehnchen/idea-php-symfony2-plugin#1277

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Routing] allow using compiled matchers and generators without dumping PHP code #28865

[Routing] allow using compiled matchers and generators without dumping PHP code #28865

nicolas-grekas commented Oct 14, 2018 •

edited

nicolas-grekas commented Oct 16, 2018

javiereguiluz left a comment

nicolas-grekas commented Jan 22, 2019

fabpot commented Jan 23, 2019

nicolas-grekas commented Jan 23, 2019

Tobion commented Feb 14, 2019

Tobion commented Feb 14, 2019

Tobion commented Feb 14, 2019

[Routing] allow using compiled matchers and generators without dumping PHP code #28865

[Routing] allow using compiled matchers and generators without dumping PHP code #28865

Conversation

nicolas-grekas commented Oct 14, 2018 • edited

nicolas-grekas commented Oct 16, 2018

javiereguiluz left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Jan 22, 2019

fabpot commented Jan 23, 2019

nicolas-grekas commented Jan 23, 2019

Tobion commented Feb 14, 2019

Tobion commented Feb 14, 2019

Tobion commented Feb 14, 2019

nicolas-grekas commented Oct 14, 2018 •

edited