Skip to content

Remove the method attribute in links#4

Merged
kimlai merged 1 commit intolrqdo:masterfrom
kimlai:remove-method
Aug 23, 2016
Merged

Remove the method attribute in links#4
kimlai merged 1 commit intolrqdo:masterfrom
kimlai:remove-method

Conversation

@kimlai
Copy link

@kimlai kimlai commented Aug 23, 2016

The current implementation uses the Router::getRouteCollection method, which loads all routes. If you use yaml to define your routes, it will parse all route files, which has a pretty big performance impact on a
large application with many routes.

This method is actually intended to be used to warm up the cache and nothing else (the result is cached in memory, but every request will read the configuration with no cache involved).

See symfony/symfony#7368 (comment) for a discussion on this topic.

We could implement a router that can do what we need efficiently, but given that we don't use the method attribute, let's just remove it completely.

Note : Blackfire says that this change leads to a -200ms improvement in an app with 260 routes.

The current implementation uses the `Router::getRouteCollection` method,
which loads all routes. If you use yaml to define your routes, it will
parse all route files, which has a pretty big performance impact on a
large application with many routes.

This method is actually intended to be used to warm up the cache and
nothing else (the result is cached in memory, but every request will
read the configuration with no cache involved).

See symfony/symfony#7368 (comment)
for a discussion on this topic.

We could implement a router that can do what we need efficiently, but
given that we don't use the `method` attribute, let's just remove it
completely.

Note : Blackfire says that this change leads to a -200ms improvement in
an app with 260 routes.
@Sinewyk
Copy link

Sinewyk commented Aug 23, 2016

👍

1 similar comment
@nclavaud
Copy link

👍

@trompette
Copy link

And by the way, the method attribute is not part of the specification.

👍

@kimlai
Copy link
Author

kimlai commented Aug 23, 2016

@trompette If it's not, how do you know which verb to use when following a link ?

@trompette
Copy link

trompette commented Aug 23, 2016

@kimlai it looks like links are designed to specify URIs not the way you interact with the resource identified by the URI.

Ref: https://apigility.org/documentation/api-primer/halprimer

@kimlai
Copy link
Author

kimlai commented Aug 23, 2016

Yeah that's what I figured after re-reading the spec.

@kimlai kimlai merged commit 4e4e8d5 into lrqdo:master Aug 23, 2016
@kimlai kimlai deleted the remove-method branch August 23, 2016 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants