amphp / http-server-router

A router for Amp's HTTP Server.
MIT License
38 stars 6 forks source link

Consider symfony/routing 4.1? #5

Closed nicolas-grekas closed 2 years ago

nicolas-grekas commented 6 years ago

It inherits from FastRoute, now faster and with more features. See https://medium.com/@nicolas.grekas/making-symfonys-router-77-7x-faster-1-2-958e3754f0e1

bwoebi commented 6 years ago

I'm a little confused about "faster". As far as I read, there are vast differences between first route, last route, failed match, dynamic case, static case, mixing dynamic&static etc.

Perhaps the chunking sizes got outdated with performance improvements related to PCRE 2, PCRE JIT or such. I think if that's the case, it should be trivial to change that upstream instead.

Regarding the features - not sure whether we really need that here - you can trivially clone the router and attach two different routes to either of them and attach it to your vhost dispatcher. The router is currently about the initial line of the HTTP Request: method and path. Everything else is already covered by other mechanisms (vhost dispatcher). So not sure whether this would just be out of scope here.

nicolas-grekas commented 6 years ago

About faster, the second part explains why we now achieve better performance: https://medium.com/@nicolas.grekas/making-symfony-router-lightning-fast-2-2-19281dcd245b About the feature set, the one gathered by the Symfony router has been accumulated over time by requests from the community, so I make the assumption it covers some real use cases. Of course that's just a suggestion, I may miss some policies/principles of the project here. That's just a proposal.

kelunik commented 6 years ago

@bwoebi We no longer have vhosts. ;-P

@nicolas-grekas I've considered that, but went with what we already had in the few years of Aerys lifetime. Seems like it requires 7.1, while currently all our projects require 7.0.

bwoebi commented 6 years ago

@kelunik I have lost a bit the orientation with all the packages - before it was rather nicely in one place. To my knowledge consensus was to split vhosts as a middleware into a separate package?

kelunik commented 6 years ago

@bwoebi They can be reimplemented, yes, but there are currently no plans for that from my or @trowski's side I think.

kelunik commented 2 years ago

It should be very simple to build another package like this based on symfony/routing. I'm glad we don't have a coupled design like in the past anymore.