Bug #16343 [Router] Too many Routes ? #16386

jelte · 2015-10-30T00:19:07Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#16343
License	MIT
Doc PR	N/A

Seems there is an issue when you have more than 7265 routes declared,
The routes are generated into the cached appDevUrlGenerator.php but php only loads the last 7265 elements of the array.

Tobion · 2015-10-30T01:54:37Z

AFAIK php arrays are only limited my memory. So what does this solve?

jelte · 2015-10-30T07:47:21Z

I know, that's what I thought as well.

yet when I checked out https://github.com/AntoineLemaire/symfony-standard/commits/2.8, and I debugged the generated appDevUrlGenerator.php file by dumping the self::$declaredRoutes there were only 7265 routes in there instead of 20000+.

well after a bit of experimenting it seems the issue is that declaring the routes in the constructor instead of the class fixes it as well.

will make a test case to illustrate the issue.

jelte · 2015-10-30T09:15:45Z

you can find an illustration of the issue at http://khepri.be/test_appDevUrlGenerator.php ( source at http://khepri.be/test_appDevUrlGenerator.php.txt )

Basicly 2 identical arrays, one defined in the class and one in the constructor.
constructor shows the correct amount while the one in the class only has 1 element.

mvrhov · 2015-10-30T13:23:29Z

# php /srv/www/test_appDevUrlGenerator.php.txt
32769 routes in constructor
32769 routes in class
# php -v
PHP 5.5.30-1.pi (cli) (built: Oct 19 2015 07:37:46)
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2015 Zend Technologies
with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies
with Xdebug v2.3.3, Copyright (c) 2002-2015, by Derick Rethans
with blackfire v1.6.0, https://blackfire.io, by Blackfireio Inc.

jelte · 2015-10-30T14:50:41Z

Seems to be an issue from php 5.6+

you can find a simplified example at https://gist.github.com/jelte/b2a9d73aec0211b07f6e

mvrhov · 2015-10-30T17:25:40Z

Then I suggest reporting this. As it probably also doesn't work in 7.0

Tobion · 2015-10-30T17:43:39Z

I confirm the bug in PHP 5.6.13. Seems pretty serious. Can somebody report it on PHP?

jelte · 2015-10-30T20:51:20Z

done, https://bugs.php.net/bug.php?id=70823

sstok · 2015-10-31T10:08:18Z

It was marked duplicate, and can't be fixed without breaking extensions.
20000+ routes? WTF! Why do you need this many?

Not using static may work also, but is a problem when you use something like ReactPHP and get allot of duplicated values in memory.

If you have this many routes, I guess one way to make it work is to separate them using a (I hope get this term right) binary-tree when the routes are split over multiple static variables (like done for the dumped matching).

ghost · 2015-11-02T02:35:00Z

Known PHP issue for PHP5.6+ but it is only fixed in PHP7+.
https://bugs.php.net/bug.php?id=68057

[2015-03-23 17:51 UTC] bwoebi@php.net
Fixing this is an ABI break (so not really possible for 5.6.x). It's fixed in master (PHP 7+) though.

jelte · 2015-11-02T14:41:46Z

As this is a bug in PHP 5.6 which won't be fixed I made it an exception for that specific PHP version and only in case you have a large number of routes.

@sstok The large number of routes are due to a large amount of locales (30+) and translated uri's (check the ticket), With large sites, without some dynamic routes it can happen.

stof · 2015-11-02T14:48:23Z

this needs to be covered by a test

SoboLAN · 2015-11-02T15:18:36Z

src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

+        $routes = $this->generateDeclaredRoutes();
+
+        // Fix for https://bugs.php.net/bug.php?id=68057 - Bug #68057 Incorrect parsing of big arrays in PHP 5.6.0
+        if (PHP_VERSION_ID >= 50600 && PHP_VERSION_ID < 50700 && count($routes) > 32750) {


Why 32750 ? Where did you get this value from ?

Also, please keep in mind that, if the size of the array is very large (maybe 50K), then count will also not work correctly. I just tested this on PHP 5.6 with an array of 56K elements. In that case, count said the array contained 23K elements.

So checking this with count won't work.... Unfortunately I can't think of an alternative implementation off-the-top of my head :( .

jelte · 2015-11-02T16:22:10Z

@stof Test has been added
@SoboLAN generateDeclaredRoutes is a string so obviously it didn't work, but counting the routes does. Also set the correct cut off number to 32769.
@sstok good point, added.

staabm · 2015-11-02T17:57:51Z

src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

@@ -38,6 +38,45 @@ public function dump(array $options = array())
            'base_class' => 'Symfony\\Component\\Routing\\Generator\\UrlGenerator',
        ), $options);

+        $routes = $this->generateDeclaredRoutes();
+
+        // Fix for https://bugs.php.net/bug.php?id=68057 - Bug #68057 Incorrect parsing of big arrays in PHP 5.6.0


Bug is contained in 5.6.* not only 5.6.0

PHP_VERSION_ID >= 50600 && PHP_VERSION_ID < 50700
=> any version between 5.6 & 5.7, comment is the title of the bug report.

the impl. is fine, just the comment is not consistent with the code.

nicolas-grekas · 2015-11-05T14:49:52Z

src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

@@ -62,6 +62,9 @@ public function __construct(RequestContext \$context, LoggerInterface \$logger =
    {
        \$this->context = \$context;
        \$this->logger = \$logger;
+        if ( null === self::\$declaredRoutes ) {


no space inside: if (null === self::\$declaredRoutes)

nicolas-grekas · 2015-11-05T14:55:32Z

👍

nicolas-grekas · 2015-11-25T12:21:04Z

Thank you @jelte.

This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #16386). Discussion ---------- Bug #16343 [Router] Too many Routes ? | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16343 | License | MIT | Doc PR | N/A Seems there is an issue when you have more than 7265 routes declared, The routes are generated into the cached appDevUrlGenerator.php but php only loads the last 7265 elements of the array. Commits ------- 0113ac3 Bug #16343 [Router] Too many Routes ?

jakzal · 2015-11-25T16:29:06Z

@nicolas-grekas for some reason this PR wasn't closed even though you merged it.

wouterj · 2015-11-25T16:31:26Z

@jakzal that's because it was rebased (on top of 2.3) while merging, so it's not a normal merge commit. In such cases, PRs are closed only when the commit makes it into the default branch (2.8) (because the commit messages includes close #xxxxx). Seems like 2.3 is not yet merged into 2.7/2.8

jakzal · 2015-11-25T16:38:04Z

@wouterj yeah, that's what I thought. Cheers!

carsonbot added the Status: Needs Review label Oct 30, 2015

jelte mentioned this pull request Oct 30, 2015

Too many Routes ? #16343

Closed

jelte force-pushed the bugfix/too-many-routes branch from 358aa72 to 98314f2 Compare October 30, 2015 07:39

jelte force-pushed the bugfix/too-many-routes branch from f744e8d to 032d386 Compare October 30, 2015 09:21

jelte force-pushed the bugfix/too-many-routes branch 2 times, most recently from 65bf924 to 8cb24cc Compare November 2, 2015 14:34

SoboLAN reviewed Nov 2, 2015
View reviewed changes

jelte force-pushed the bugfix/too-many-routes branch 3 times, most recently from 56ebc5b to 058f5fa Compare November 2, 2015 16:18

staabm reviewed Nov 2, 2015
View reviewed changes

jelte force-pushed the bugfix/too-many-routes branch 3 times, most recently from 17991e0 to 0ee6033 Compare November 5, 2015 14:41

nicolas-grekas reviewed Nov 5, 2015
View reviewed changes

Bug symfony#16343 [Router] Too many Routes ?

35dfe3e

jelte force-pushed the bugfix/too-many-routes branch from 0ee6033 to 35dfe3e Compare November 5, 2015 14:51

fabpot closed this Nov 25, 2015

This was referenced Nov 30, 2015

Release v2.8.0 #16763

Merged

Release v3.0.0 #16766

Merged

This was referenced Dec 26, 2015

Release v2.3.36 #17141

Merged

Release v2.7.8 #17142

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug #16343 [Router] Too many Routes ? #16386

Bug #16343 [Router] Too many Routes ? #16386

jelte commented Oct 30, 2015

Tobion commented Oct 30, 2015

jelte commented Oct 30, 2015

jelte commented Oct 30, 2015

mvrhov commented Oct 30, 2015

jelte commented Oct 30, 2015

mvrhov commented Oct 30, 2015

Tobion commented Oct 30, 2015

jelte commented Oct 30, 2015

sstok commented Oct 31, 2015

ghost commented Nov 2, 2015

jelte commented Nov 2, 2015

stof commented Nov 2, 2015

SoboLAN Nov 2, 2015

jelte commented Nov 2, 2015

staabm Nov 2, 2015

jelte Nov 3, 2015

staabm Nov 3, 2015

nicolas-grekas Nov 5, 2015

nicolas-grekas commented Nov 5, 2015

nicolas-grekas commented Nov 25, 2015

jakzal commented Nov 25, 2015

wouterj commented Nov 25, 2015

jakzal commented Nov 25, 2015

Bug #16343 [Router] Too many Routes ? #16386

Bug #16343 [Router] Too many Routes ? #16386

Conversation

jelte commented Oct 30, 2015

Tobion commented Oct 30, 2015

jelte commented Oct 30, 2015

jelte commented Oct 30, 2015

mvrhov commented Oct 30, 2015

jelte commented Oct 30, 2015

mvrhov commented Oct 30, 2015

Tobion commented Oct 30, 2015

jelte commented Oct 30, 2015

sstok commented Oct 31, 2015

ghost commented Nov 2, 2015

jelte commented Nov 2, 2015

stof commented Nov 2, 2015

SoboLAN Nov 2, 2015

Choose a reason for hiding this comment

jelte commented Nov 2, 2015

staabm Nov 2, 2015

Choose a reason for hiding this comment

jelte Nov 3, 2015

Choose a reason for hiding this comment

staabm Nov 3, 2015

Choose a reason for hiding this comment

nicolas-grekas Nov 5, 2015

Choose a reason for hiding this comment

nicolas-grekas commented Nov 5, 2015

nicolas-grekas commented Nov 25, 2015

jakzal commented Nov 25, 2015

wouterj commented Nov 25, 2015

jakzal commented Nov 25, 2015