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

[Enhancement] RouteCollection and UrlMatcher Performance #18145

Closed
leonelvsc opened this issue Mar 13, 2016 · 5 comments
Closed

[Enhancement] RouteCollection and UrlMatcher Performance #18145

leonelvsc opened this issue Mar 13, 2016 · 5 comments

Comments

@leonelvsc
Copy link

Hi all, first than anything my english it's not so good so i apologize and this is just a suggestion.

Reading the RouteCollection class i saw that currently you store the routes in an array and then in the UrlMatcher you iterate the routes array until you find a match.

My suggestion is that you use a Hash structure in the array for optimizing performance , i.e if the app has 100000 routes that loop for in the UrlMatcher class has to iterate all the array (in the worst case).

Maybe we can research if it's possible to store routes in the following way.

For the RouteCollection class add a hashedRoutes array

     /**
     * @var Route[]
     */
    private $routes = array();

    /**
     * @var Route[]
     */
    private $hashedRoutes = array();

Then when you add a route to the collection you build some sort of hash

i.e

    /**
     * Adds a route.
     *
     * @param string $name  The route name
     * @param Route  $route A Route instance
     */
    public function add($name, Route $route)
    {
        unset($this->routes[$name]);

        $compiledRoute = $route->compile();
        $prefix = $compiledRoute->getStaticPrefix();
        $index = substr($prefix, 0, strpos($prefix, '/'));

        $this->hashedRoutes[$index][] = $route;

        $this->routes[$name] = $route;
    }

then we could add the following method to the RouteCollection

    /**
     *
     * @param $pathInfo
     */
    public function getWithPrefix($pathInfo)
    {
        $index = substr($pathInfo, 0, strpos($pathInfo, '/'));
        return (isset($this->hashedRoutes[$index]))? $this->hashedRoutes[$index] : [];
    }

Of course that for this to work we would have to regenerate the hash in several methods of the RouteCollection i.e the addPrefix method

Then if we store the Routes in a Hash inside the RouteCollection the UrlMatcher class can take advantage of this, and just itarate over the routes that matches the hash , i.e

    protected function matchCollection($pathinfo, RouteCollection $routes)
    {
        foreach ($routes->getWithPrefix($pathinfo) as $name => $route) {
@leonelvsc leonelvsc changed the title RouteCollection and UrlMatcher Performance [Enhancement] [Enhancement] RouteCollection and UrlMatcher Performance Mar 13, 2016
@sstok
Copy link
Contributor

sstok commented Mar 13, 2016

Does this also happen with the dumped UrlMatcher? Or is this optimization for when you don't dump the UrlMatcher.

@leonelvsc
Copy link
Author

I think that the only use for this enhancement is when you don't dump the UrlMatcher. If I'm not mistaken sf group routes when you dump right?

@javiereguiluz
Copy link
Member

@leonelvsc thanks for opening this issue to try to improve Symfony's UrlMatcher.

Since Symfony dumps the UrlMatcher to PHP code even in the dev environment, I'd like to ask you in which scenarios do you think this improvement could be used. The Routing component is a critical piece for Symfony framework, so we are always very reluctant to change anything on it (except bug fixes). Thanks!

@leonelvsc
Copy link
Author

Well as i said before the only useful case is when you don't wanna dump the UrlMatcher , ie: another project that is using this component but not the symfony framework.

But taking in count that is a critical piece of the framework it self , and this is not a major improvement , this proposal has no sense at all.

@javiereguiluz
Copy link
Member

I'm closing this as "fixed". Although we didn't implement this exact proposal, in Symfony 3.3 we optimized both the UrlMatcher and the UrlGenerator. See https://symfony.com/blog/new-in-symfony-3-3-faster-routing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants