Skip to content
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

perf: RouteCollection $routes optimization #7175

Merged
merged 11 commits into from
Jun 28, 2023
Merged

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jan 25, 2023

Needs #7174
This PR should go to 4.4 branch.

Description
Ref #6889 (comment)

  • change RouteCollection::$routes structure

Before:

     * [
     *     verb => [
     *         routeName => [
     *             'route' => [
     *                 routeKey(regex) => handler,
     *             ],
     *         ],
     *     ],
     *     // redirect route
     *     '*' => [
     *         routeName => [
     *             'route' => [
     *                 routeKey(regex)(from) => [routeKey(regex)(to) => handler],
     *             ],
     *             'redirect' => statusCode,
     *         ]
     *     ],
     * ]

After:

     * [
     *     verb => [
     *         routeKey(regex) => [
     *             'name'    => routeName
     *             'handler' => handler,
     *             'from'    => from,
     *         ],
     *     ],
     *     // redirect route
     *     '*' => [
     *          routeKey(regex)(from) => [
     *             'name'     => routeName
     *             'handler'  => [routeKey(regex)(to) => handler],
     *             'from'     => from,
     *             'redirect' => statusCode,
     *         ],
     *     ],
     * ]
  • add RouteCollection::$routesName

Benchmark
app/Config/Routes.php:

helper('text');

for ($i = 0; $i < 5000; $i++) {
    $from[] = random_string('alpha');
}

timer('register 5001 routes');

for ($i = 0; $i < 5000; $i++) {
    $routes->get($from[$i], 'Home::index' . $i);
}
$routes->get('last', 'Last::last');
timer('register 5001 routes');
d(timer()->getElapsedTime('register 5001 routes'));

$_SERVER['REQUEST_METHOD'] = 'GET';
$requet                    = \Config\Services::incomingrequest(null, false);
$router                    = new \CodeIgniter\Router\Router($routes, $requet);

timer('find last route');
$router->handle('last');
timer('find last route');
dd(timer()->getElapsedTime('find last route'));

register 5001 routes:

 v4.2.7:  0.0243 seconds
 v4.3.1:  10.2108
This PR:  0.0259

find last route:

 v4.2.7:  0.0417 seconds
 v4.3.1:  0.0456
This PR:  0.0458

macOS 12.6.6
PHP 8.1.20 (cli) (built: Jun 8 2023 20:06:33) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.20, Copyright (c) Zend Technologies
with Zend OPcache v8.1.20, Copyright (c), by Zend Technologies

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Jan 25, 2023
@kenjis kenjis marked this pull request as draft January 25, 2023 07:02
@kenjis
Copy link
Member Author

kenjis commented Jan 27, 2023

With Xdebug turned off, updated the benchmarks.

@samsonasik
Copy link
Member

It seems the changes are only internal, and it doesn't change the usage. I think it can go to the next 4.3.2 release with add note for change structure in case user utilize the structure of it.

@kenjis kenjis force-pushed the perf-router branch 2 times, most recently from 25d7c45 to f1ec2db Compare February 1, 2023 11:06
@kenjis kenjis changed the base branch from develop to 4.4 February 1, 2023 11:06
@kenjis kenjis added the 4.4 label Feb 1, 2023
@kenjis kenjis added the docs needed Pull requests needing documentation write-ups and/or revisions. label Jun 10, 2023
@kenjis kenjis removed the docs needed Pull requests needing documentation write-ups and/or revisions. label Jun 27, 2023
@kenjis
Copy link
Member Author

kenjis commented Jun 27, 2023

Updated the benchmark and added docs.

@kenjis kenjis marked this pull request as ready for review June 27, 2023 00:42
@datamweb
Copy link
Contributor

@kenjis I do not have a correct understanding of this PR. Will the shield be affected?
https://github.com/codeigniter4/shield/blob/3412fac1a2a59d80ddca5c34812e2b2c24c113ec/src/Auth.php#L109-L125

@kenjis
Copy link
Member Author

kenjis commented Jun 27, 2023

@datamweb No. This is an internal change to theRouteCollection.
Unless you extend the class, nothing will change.

@kenjis kenjis merged commit c02bae9 into codeigniter4:4.4 Jun 28, 2023
@kenjis kenjis deleted the perf-router branch June 28, 2023 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.4 breaking change Pull requests that may break existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants