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

[Routing] fix perf issue when dumping large number of routes #30058

Merged
merged 1 commit into from Feb 7, 2019

Conversation

Projects
None yet
3 participants
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 1, 2019

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

In my reproducer, dumping 12k routes goes from 40s to 3s without xdebug, and from 50s to 12s with xdebug.

There is a lower level issue which is that strpos is called 16M times, but that's still a lot faster than calling preg_match 16M times. Reducing the number of checks is certainly possible, but that would be more involving. This could happen on master if someone is up to dig into it.

@javiereguiluz
Copy link
Member

javiereguiluz left a comment

Nice!

As a minor suggestion, I'd rename some variables:

  • $prefix -> $staticPrefix
  • $p -> $prefix

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:route-perf branch from 1e4ee16 to 872efe5 Feb 7, 2019

@nicolas-grekas nicolas-grekas merged commit 872efe5 into symfony:4.2 Feb 7, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Feb 7, 2019

bug #30058 [Routing] fix perf issue when dumping large number of rout…
…es (nicolas-grekas)

This PR was merged into the 4.2 branch.

Discussion
----------

[Routing] fix perf issue when dumping large number of routes

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

In my reproducer, dumping 12k routes goes from 40s to 3s without xdebug, and from 50s to 12s with xdebug.

There is a lower level issue which is that `strpos` is called 16M times, but that's still a lot faster than calling `preg_match` 16M times. Reducing the number of checks is certainly possible, but that would be more involving. This could happen on master if someone is up to dig into it.

Commits
-------

872efe5 [Routing] fix perf issue when dumping large number of routes

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:route-perf branch Feb 7, 2019

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