Problem/Motivation
We saw in #2470679: [meta] Identify necessary performance optimizations for common profiling scenarios that routing for e.g. /node/1 is quite a bit slower in D8. A simple optimization we could do, is to handle the most frequently requested routes in PHP code only (i.e. not talking to the DB), and return early. Thus, only using the DB for less commonly accessed routes.
Proposed resolution
Refactor the routing process so we have a chained route provider, and simplify the remaining flow to removed the chained router, etc.
Comments
Comment #1
catchAsked this on irc yesterday. What happens if:
node/%node is in PHP.
node/1 is registered explicitly as a separate route, and is not in PHP.
(node/add and node/%node are probably a better example, except that node/add wouldn't be a valid path for node/%node).
That doesn't make this impossible, but it might mean we have a restriction that all paths in a top level directory/path root are either in PHP or not.
Also it's a different optimization but #2406117: potentially cache parts of routing would be a higher-level cache we could add (compared to this) which would also help with route lookup, although that has its own complexities too.
Comment #2
dawehnerSymfony kinda solves that by adding a regular expression for the %node example.
In general this seems to be an optimization, that is tricky to be implemented as a generic case, but rather could be applied really easy on
custom base, as you have more knowledge available.
If we wanna do that, maybe we could write a decorator for RouteProvider, that people on custom sites could adapt things more easy ...
note: We also want to make it possible to reuse that chaining for other routing implementations like mongo.
Comment #3
larowlanPity the dump to .htaccess option doesn't work
Comment #4
fabianx commentedWe can still do this when we compile routes statically to PHP on runtime without problems ...
As we then can check for all path-prefixes and group all same matching paths together ...
Comment #5
pwolanin commentedSo - IMHO the issue here is really to change the architecture of routing a bit to make ths easy, not necessarily implement the PHP dumping
Comment #6
pwolanin commentedComment #7
pwolanin commentedComment #8
pwolanin commentedComment #9
Crell commentedI got a bit lost part way through this thread... why subclass anything or wrap anything? I'm fully on board with adding an earlier DB-free router; ChainRouter is in there for exactly that reason. But piling more wrappers onto the RouteProvider doesn't seem like a wise idea to me. That's the wrong place to add this sort of flexibility. Honestly I'm a little concerned that we've put enough into the current router as is that leveraging ChainRouter may be difficult.
Although part of me is quite curious what would happen if we swapped out the RoutingProvider with one that does PHP dump for everything. I have no idea what would happen, honestly. :-) It might be worth investigating.
Comment #10
dawehnerWell @crell, the problem is that the chained router architecture is just not exactly what we need ... as it would entirely bypass
our upcasting logic, I mean sure, in case we have a really limited list of routes, we might be able to just hardcode the entity loading, but its something which was not taken into account, if you ask me.
Comment #11
Crell commenteddawehner: Yep, that's exactly the sort of thing I mean by "Honestly I'm a little concerned that we've put enough into the current router as is that leveraging ChainRouter may be difficult.". There's a couple of ways to addressing that:
1) For those routes we put in the "early/fast" router, we hard code appropriate logic. This is probably faster, but more brittle and less site-flexible.
2) Use another DynamicRouter instance for the early/fast router, too, and wire *the exact same enhancers and filters and stuff" into that one. Stateless services make this easy. :-) If there's a filter or enhancer that we can't reuse that way, it's because we did it wrong. This would be the more robust option, but may not buy us as much on performance since we'd still be loading many of the same services.
Also remember that the way ChainRouter works, each "failed" router results in a 404 exception. Creating and throwing an exception is non-free in PHP; it costs several times a function call.
Unfortunately I don't know any way to determine the best approach here other than writing it a bunch of different ways and benchmarking the hell out of it. :-)
Comment #12
catchComment #14
dawehnerThis could be also a usecase for an experimental module
Comment #16
tim.plunkettNot sure how major this is anymore. Also note we're removing ChainRouter in #2810303: Reunite the router: One router to rule them all
Comment #17
dawehnerIMHO such a module is most of the time custom. You can make bigger assumptions there so its actually worth it. Generalizations will be hard, given that upcasting for example might be needed for specific usecases.
Comment #19
dawehnerI'm honestly not sure its really worth even trying that.
Comment #20
wim leers